On Thu, Mar 8, 2012 at 5:55 PM, Patrick Ohly <patrick.ohly(a)intel.com> wrote:
On Thu, 2012-03-08 at 17:18 +0100, Chris Kühl wrote:
> On Thu, Mar 8, 2012 at 4:43 PM, Patrick Ohly <patrick.ohly(a)intel.com> wrote:
> > On Wed, 2012-03-07 at 16:54 +0100, Chris Kühl wrote:
> >> Now that you've merged for-master/new-master into master we are
> >> rebasing onto master.
> >
> > How is that going?
> >
>
> It's been rebased and we've squashed the changes into about a dozen
> commits. The tests seem to give us the same results as before the
> rebase. See the concurrent-sync-sessions-rebased-pre-cleanup branch
> for the rebased and squashed changes. I'm putting the final touches on
> the cleanup know.
057a19f18c600fe4158fd06087aed10b44f20c4e
syncevo: Added methods to add and get environment variables to ForkExec
The parent may need to pass additional environment variables to the
child process.
This commit adds an Add method to the parent process and a Get method
for the child process.
+ /*
+ * Return the value for the environment variable name or and empty
+ * string is not found or set to ForkExecEnvVarEmpty.
+ */
+ std::string getEnvVar(const std::string &name);
Why is this ForkExecEnvVarEmpty necessary?
Ah, originally I wanted to use the just the fact that the variable
existed without setting a value but some value needs to be set for env
variables. However, somewhere along the line I changed my mind. I'll
get rid of this.
Note that there is already a GetEnv() in util.h which returns an
empty
std::string when getenv() returns NULL.
Thanks, I wasn't aware. I'll change it to use that.
+std::string ForkExecChild::getEnvVar(const std::string &name)
+{
+ gchar *pValue = getenv(name.c_str());
+ if(!pValue || boost::iequals(pValue, ForkExecEnvVarEmpty)) {
+ return std::string();
+ } else {
+ return std::string(pValue);
+
getenv() returns "char *", not "gchar *". I don't know where that
makes
a difference, but if it is guaranteed to be the same, why introduce
gchar? That implies to me that there might be a difference somewhere.
You're right. Moot point now though.
d85da736dec0975962b610e5c3b0b0dc52b612b3
ForkExec.cpp: Fix the build without --enable-dbus-service.
The .h file's contents are inside an ifdef, so the implementation in
.cpp should be too.
--------------------------- src/syncevo/ForkExec.cpp ---------------------------
index 0395c33..1f71331 100644
@@ -18,8 +18,10 @@
*/
#include "ForkExec.h"
+#if defined(HAVE_GLIB) && defined(DBUS_SERVICE)
+
SE_BEGIN_CXX
#if defined(HAVE_GLIB)
@@ -292,4 +294,5 @@ const char *ForkExecChild::getParentDBusAddress()
#endif // HAVE_GLIB
SE_END_CXX
+#endif // HAVE_GLIB
The if and endif don't match, and HAVE_GLIB is tested twice. I suggest
+#if defined(HAVE_GLIB) && defined(DBUS_SERVICE)
...
-#if defined(HAVE_GLIB)
...
-#endif // HAVE_GLIB
...
+#endif // HAVE_GLIB && DBUS_SERVICE
Yeah, just saw that too.
dd0a4ab2d584004095f5f0e7cd30770ae3d952c8
Add some common DBus callbacks.
These callbacks will be used once sync sessions run in their own
process.
nullCb: a function that does nothing.
counterCb: a function that calls given callback when given counter
drops to zero.
Example use:
...
The documentation should be in comments in the header file, not in the
commit message. That way it is more readily available when reading the
source code in the future.
Note that global functions normally start with a a capital letter, to
distinguish them from member methods, which start with a lower capital.
I'll make these changes and squash them into the for-review branch.
Cheers,
Chris