Do not attempt to call child_setup on Windows

child_setup() is defined to be called after fork() and before exec(),
but Windows' process model does not have fork(): the equivalent of
those two operations is a single CreateProcess() call. This means
that there is no point at which we could call child_setup() and
have it affect only the child's process-global state. At the point
where it is currently executed, it affects the parent's process-global
state instead, which would be actively harmful if we used any
child_setup() function that was not a no-op on Windows.

The equivalent function in GLib, g_spawn_async_with_pipes(), documents
child_setup() as unused on Windows. Do the same here.

In practice, our only use of child_setup() outside tests
is #ifdef DBUS_UNIX anyway, so this change has no practical effect
right now.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=85857
Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>

diff --git a/dbus/dbus-spawn-win.c b/dbus/dbus-spawn-win.c
index c58bf3cd376930f779a0548d0a5c14f1c5674aa9..43fafd5ad5bcff0fedf8852a9da8ee2e0430c0da 100644 (file)
--- a/dbus/dbus-spawn-win.c
+++ b/dbus/dbus-spawn-win.c
@@ -69,8 +69,6 @@ struct DBusBabysitter
 #endif
 
     char *log_name;
-    DBusSpawnChildSetupFunc child_setup;
-    void *user_data;
 
     int argc;
     char **argv;
@@ -588,12 +586,6 @@ babysitter (void *parameter)
   PING();
   _dbus_babysitter_ref (sitter);
 
-  if (sitter->child_setup)
-    {
-      PING();
-      (*sitter->child_setup) (sitter->user_data);
-    }
-
   _dbus_verbose ("babysitter: spawning %s\n", sitter->log_name);
 
   PING();
@@ -648,8 +640,8 @@ _dbus_spawn_async_with_babysitter (DBusBabysitter           **sitter_p,
                                    const char                *log_name,
                                    char                     **argv,
                                    char                     **envp,
-                                   DBusSpawnChildSetupFunc    child_setup,
-                                   void                      *user_data,
+                                   DBusSpawnChildSetupFunc    child_setup _DBUS_GNUC_UNUSED,
+                                   void                      *user_data _DBUS_GNUC_UNUSED,
                                    DBusError                 *error)
 {
   DBusBabysitter *sitter;
@@ -669,9 +661,6 @@ _dbus_spawn_async_with_babysitter (DBusBabysitter           **sitter_p,
       return FALSE;
     }
 
-  sitter->child_setup = child_setup;
-  sitter->user_data = user_data;
-
   sitter->log_name = _dbus_strdup (log_name);
   if (sitter->log_name == NULL && log_name != NULL)
     {

diff --git a/dbus/dbus-spawn.c b/dbus/dbus-spawn.c
index ddd254dc5c8feaf1971afe8b24f1b89c7b71cf06..e591e69009bee9c1e3c5277c4a18bcee314641a7 100644 (file)
--- a/dbus/dbus-spawn.c
+++ b/dbus/dbus-spawn.c
@@ -1188,9 +1188,14 @@ babysit (pid_t grandchild_pid,
 }
 
 /**
- * Spawns a new process. The child_setup
- * function is passed the given user_data and is run in the child
- * just before calling exec().
+ * Spawns a new process.
+ *
+ * On Unix platforms, the child_setup function is passed the given
+ * user_data and is run in the child after fork() but before calling exec().
+ * This can be used to change uid, resource limits and so on.
+ * On Windows, this functionality does not fit the multi-processing model
+ * (Windows does the equivalent of fork() and exec() in a single API call),
+ * and the child_setup function and its user_data are ignored.
  *
  * Also creates a "babysitter" which tracks the status of the
  * child process, advising the parent if the child exits.