[Swift-commit] cog r3936
swift at ci.uchicago.edu
swift at ci.uchicago.edu
Fri Jun 20 18:20:03 CDT 2014
------------------------------------------------------------------------
r3936 | timgarmstrong | 2014-06-20 18:17:27 -0500 (Fri, 20 Jun 2014) | 1 line
Add a dose of pedantry to the coaster C client interfaces based on my attempts to understand memory lifetime/ownership. Document in a few cases and add const qualifiers where appropriate in other cases.
------------------------------------------------------------------------
Index: modules/provider-coaster-c-client/src/CoasterChannel.h
===================================================================
--- modules/provider-coaster-c-client/src/CoasterChannel.h (revision 3935)
+++ modules/provider-coaster-c-client/src/CoasterChannel.h (working copy)
@@ -103,7 +103,7 @@
void send(int tag, Buffer* buf, int flags, ChannelCallback* cb);
CoasterClient* getClient();
- string& getURL();
+ const string& getURL() const;
void checkHeartbeat();
Index: modules/provider-coaster-c-client/src/CoasterLoop.h
===================================================================
--- modules/provider-coaster-c-client/src/CoasterLoop.h (revision 3935)
+++ modules/provider-coaster-c-client/src/CoasterLoop.h (working copy)
@@ -51,7 +51,12 @@
virtual ~CoasterLoop();
void start();
void stop();
-
+
+ /*
+ * Add a channel for the loop to monitor.
+ * Ownership of the channel is retained by the caller.
+ * Must be removed later by a call to removeChannel().
+ */
void addChannel(CoasterChannel* channel);
void removeChannel(CoasterChannel* channel);
void addSockets();
Index: modules/provider-coaster-c-client/src/CoasterClient.cpp
===================================================================
--- modules/provider-coaster-c-client/src/CoasterClient.cpp (revision 3935)
+++ modules/provider-coaster-c-client/src/CoasterClient.cpp (working copy)
@@ -167,7 +167,7 @@
delete cmd;
}
-void CoasterClient::updateJobStatus(string& remoteJobId, JobStatus* status) { Lock::Scoped l(lock);
+void CoasterClient::updateJobStatus(const string& remoteJobId, JobStatus* status) { Lock::Scoped l(lock);
if (remoteJobIdMapping.count(remoteJobId) == 0) {
LogWarn << "Received job status notification for unknown job (" << remoteJobId << "): " << status << endl;
}
@@ -259,12 +259,12 @@
}
}
-void CoasterClient::waitForJob(Job& job) { Lock::Scoped l(lock);
+void CoasterClient::waitForJob(const Job& job) { Lock::Scoped l(lock);
while (jobs.count(&(job.getIdentity())) != 0) {
cv.wait(lock);
}
}
-string& CoasterClient::getURL() {
+const string& CoasterClient::getURL() {
return URL;
}
Index: modules/provider-coaster-c-client/src/JobStatus.cpp
===================================================================
--- modules/provider-coaster-c-client/src/JobStatus.cpp (revision 3935)
+++ modules/provider-coaster-c-client/src/JobStatus.cpp (working copy)
@@ -44,7 +44,7 @@
init(UNSUBMITTED, time(NULL), NULL, NULL);
}
-JobStatusCode JobStatus::getStatusCode() {
+JobStatusCode JobStatus::getStatusCode() const {
return statusCode;
}
@@ -52,7 +52,7 @@
return stime;
}
-string* JobStatus::getMessage() {
+const string* JobStatus::getMessage() const {
return message;
}
Index: modules/provider-coaster-c-client/src/Job.h
===================================================================
--- modules/provider-coaster-c-client/src/Job.h (revision 3935)
+++ modules/provider-coaster-c-client/src/Job.h (working copy)
@@ -27,14 +27,16 @@
* TODO: document expectations about lifetime of strings.
* It seems very easy for client code to accidentally
* pass in a pointer to a stack-allocated string or a
- * string with a shorter lifetime than the job.
+ * string with a shorter lifetime than the job. Would
+ * it work to just store them by value and have zero-length
+ * be equivalent to NULL. Are zero-length strings meaningful?
*/
vector<string*>* arguments;
string* directory;
string* stdinLocation;
string* stdoutLocation;
string* stderrLocation;
- string jobManager;
+ string jobManager;
map<string, string>* env;
map<string, string>* attributes;
@@ -49,41 +51,41 @@
Job(const string &executable);
virtual ~Job();
- string& getExecutable();
+ const string& getExecutable() const;
/*
Get the job identity. The identity is a unique string
that is assigned to the job object upon construction
and does not change over it's lifetime.
*/
- string& getIdentity();
+ const string& getIdentity() const;
vector<string*>* getArguments();
void addArgument(string& arg);
void addArgument(const char* arg);
- string* getDirectory();
+ const string* getDirectory() const;
void setDirectory(string& directory);
- string* getStdinLocation();
+ const string* getStdinLocation() const;
void setStdinLocation(string& loc);
- string* getStdoutLocation();
+ const string* getStdoutLocation() const;
void setStdoutLocation(string& loc);
- string* getStderrLocation();
+ const string* getStderrLocation() const;
void setStderrLocation(string& loc);
- string getJobManager();
+ const string &getJobManager() const;
void setJobManager(string jm);
void setJobManager(const char *jm);
map<string, string>* getEnv();
- string* getEnv(string name);
+ const string* getEnv(string name) const;
void setEnv(string name, string value);
map<string, string>* getAttributes();
- string* getAttribute(string name);
+ const string* getAttribute(string name);
void setAttribute(string name, string value);
vector<StagingSetEntry>* getStageIns();
@@ -95,10 +97,7 @@
vector<string>* getCleanups();
void addCleanup(string cleanup);
- string* getStderr();
- string* getStdout();
-
- JobStatus* getStatus();
+ const JobStatus* getStatus() const;
void setStatus(JobStatus* status);
};
Index: modules/provider-coaster-c-client/src/CoasterChannel.cpp
===================================================================
--- modules/provider-coaster-c-client/src/CoasterChannel.cpp (revision 3935)
+++ modules/provider-coaster-c-client/src/CoasterChannel.cpp (working copy)
@@ -326,8 +326,8 @@
delete cmd;
}
-string& CoasterChannel::getURL() {
- return getClient()->getURL();
+const string& CoasterChannel::getURL() const {
+ return client->getURL();
}
/*
Index: modules/provider-coaster-c-client/src/CoasterClient.h
===================================================================
--- modules/provider-coaster-c-client/src/CoasterClient.h (revision 3935)
+++ modules/provider-coaster-c-client/src/CoasterClient.h (working copy)
@@ -63,15 +63,31 @@
void stop();
void setOptions(Settings& settings);
+
+ /*
+ * Submit a job. The job should have been filled in with
+ * all properties. The ownership of the job object stays
+ * with the caller, but this client will retain a reference
+ * to the job until done jobs are purged.
+ */
void submit(Job& job);
- void waitForJob(Job& job);
+ /*
+ * Wait for job to be done. Upon completion no actions
+ * are taken: job must be purged from client explicitly.
+ */
+ void waitForJob(const Job& job);
+
+ /*
+ * Return a list of done jobs and remove references from this
+ * client.
+ */
list<Job*>* getAndPurgeDoneJobs();
void waitForJobs();
- void updateJobStatus(string& jobId, JobStatus* status);
+ void updateJobStatus(const string &jobId, JobStatus* status);
- string& getURL();
+ const string& getURL();
void errorReceived(Command* cmd, string* message, RemoteCoasterException* details);
void replyReceived(Command* cmd);
Index: modules/provider-coaster-c-client/src/Job.cpp
===================================================================
--- modules/provider-coaster-c-client/src/Job.cpp (revision 3935)
+++ modules/provider-coaster-c-client/src/Job.cpp (working copy)
@@ -32,7 +32,7 @@
status = NULL;
}
-string& Job::getIdentity() {
+const string& Job::getIdentity() const {
return identity;
}
@@ -47,7 +47,7 @@
arguments->push_back(new string(arg));
}
-string& Job::getExecutable() {
+const string& Job::getExecutable() const {
return executable;
}
@@ -55,7 +55,7 @@
addArgument(*(new string(arg)));
}
-string* Job::getDirectory() {
+const string* Job::getDirectory() const {
return directory;
}
@@ -63,7 +63,7 @@
directory = &pdirectory;
}
-string* Job::getStdinLocation() {
+const string* Job::getStdinLocation() const {
return stdinLocation;
}
@@ -71,7 +71,7 @@
stdinLocation = &loc;
}
-string* Job::getStdoutLocation() {
+const string* Job::getStdoutLocation() const {
return stdoutLocation;
}
@@ -79,7 +79,7 @@
stdoutLocation = &loc;
}
-string* Job::getStderrLocation() {
+const string* Job::getStderrLocation() const {
return stderrLocation;
}
@@ -87,7 +87,7 @@
stderrLocation = &loc;
}
-string Job::getJobManager() {
+const string& Job::getJobManager() const {
return jobManager;
}
@@ -104,7 +104,7 @@
return env;
}
-string* Job::getEnv(string name) {
+const string* Job::getEnv(string name) const {
if (env == NULL) {
return NULL;
}
@@ -131,7 +131,7 @@
return attributes;
}
-string* Job::getAttribute(string name) {
+const string* Job::getAttribute(string name) {
if (attributes == NULL) {
return NULL;
}
@@ -187,15 +187,7 @@
cleanups->push_back(cleanup);
}
-string* Job::getStderr() {
- return stderr;
-}
-
-string* Job::getStdout() {
- return stdout;
-}
-
-JobStatus* Job::getStatus() {
+const JobStatus* Job::getStatus() const {
return status;
}
Index: modules/provider-coaster-c-client/src/JobStatus.h
===================================================================
--- modules/provider-coaster-c-client/src/JobStatus.h (revision 3935)
+++ modules/provider-coaster-c-client/src/JobStatus.h (working copy)
@@ -47,9 +47,9 @@
JobStatus(JobStatusCode statusCode);
JobStatus();
virtual ~JobStatus();
- JobStatusCode getStatusCode();
+ JobStatusCode getStatusCode() const;
time_t getTime();
- string* getMessage();
+ const string* getMessage() const;
RemoteCoasterException* getException();
const JobStatus* getPreviousStatus();
void setPreviousStatus(JobStatus* prev);
Index: modules/provider-coaster-c-client/src/JobSubmitCommand.cpp
===================================================================
--- modules/provider-coaster-c-client/src/JobSubmitCommand.cpp (revision 3935)
+++ modules/provider-coaster-c-client/src/JobSubmitCommand.cpp (working copy)
@@ -5,8 +5,8 @@
using namespace std;
-void add(string& ss, const char* key, string* value);
-void add(string& ss, const char* key, string& value);
+void add(string& ss, const char* key, const string* value);
+void add(string& ss, const char* key, const string& value);
void add(string& ss, const char* key, const char* value);
void add(string& ss, const char* key, const char* value, int n);
@@ -75,13 +75,13 @@
addOutData(Buffer::wrap(ss));
}
-void add(string& ss, const char* key, string* value) {
+void add(string& ss, const char* key, const string* value) {
if (value != NULL) {
add(ss, key, value->data(), value->length());
}
}
-void add(string& ss, const char* key, string& value) {
+void add(string& ss, const char* key, const string& value) {
add(ss, key, value.data(), value.length());
}
More information about the Swift-commit
mailing list