include: uv_spawn takes const uv_process_options_t*

Passing or returning structs as values makes life hard for people that work with libuv through a foreign function interface. Switch to a pointer-based approach. Fixes #684.
/ · Sep 1, 2013 · 8c6ea10 · 8c6ea10
1 parent 263da51
commit 8c6ea10
Show file tree

Hide file tree

Showing 10 changed files with 84 additions and 79 deletions.
diff --git a/include/uv.h b/include/uv.h
@@ -1502,8 +1502,9 @@ struct uv_process_s {
 };
 
 /* Initializes uv_process_t and starts the process. */
-UV_EXTERN int uv_spawn(uv_loop_t*, uv_process_t*,
-    uv_process_options_t options);
+UV_EXTERN int uv_spawn(uv_loop_t* loop,
+                       uv_process_t* handle,
+                       const uv_process_options_t* options);
 
 
 /*

diff --git a/src/unix/process.c b/src/unix/process.c
@@ -183,7 +183,7 @@ int uv__make_pipe(int fds[2], int flags) {
 
 
 /*
- * Used for initializing stdio streams like options.stdin_stream. Returns
+ * Used for initializing stdio streams like options->stdin_stream. Returns
  * zero on success.
  */
 static int uv__process_init_stdio(uv_stdio_container_t* container, int fds[2]) {
@@ -270,15 +270,15 @@ static void uv__write_int(int fd, int val) {
 }
 
 
-static void uv__process_child_init(uv_process_options_t options,
+static void uv__process_child_init(const uv_process_options_t* options,
                                    int stdio_count,
                                    int (*pipes)[2],
                                    int error_fd) {
   int close_fd;
   int use_fd;
   int fd;
 
-  if (options.flags & UV_PROCESS_DETACHED)
+  if (options->flags & UV_PROCESS_DETACHED)
     setsid();
 
   for (fd = 0; fd < stdio_count; fd++) {
@@ -313,29 +313,29 @@ static void uv__process_child_init(uv_process_options_t options,
       uv__nonblock(fd, 0);
   }
 
-  if (options.cwd && chdir(options.cwd)) {
+  if (options->cwd != NULL && chdir(options->cwd)) {
     uv__write_int(error_fd, -errno);
     perror("chdir()");
     _exit(127);
   }
 
-  if ((options.flags & UV_PROCESS_SETGID) && setgid(options.gid)) {
+  if ((options->flags & UV_PROCESS_SETGID) && setgid(options->gid)) {
     uv__write_int(error_fd, -errno);
     perror("setgid()");
     _exit(127);
   }
 
-  if ((options.flags & UV_PROCESS_SETUID) && setuid(options.uid)) {
+  if ((options->flags & UV_PROCESS_SETUID) && setuid(options->uid)) {
     uv__write_int(error_fd, -errno);
     perror("setuid()");
     _exit(127);
   }
 
-  if (options.env) {
-    environ = options.env;
+  if (options->env != NULL) {
+    environ = options->env;
   }
 
-  execvp(options.file, options.args);
+  execvp(options->file, options->args);
   uv__write_int(error_fd, -errno);
   perror("execvp()");
   _exit(127);
@@ -344,7 +344,7 @@ static void uv__process_child_init(uv_process_options_t options,
 
 int uv_spawn(uv_loop_t* loop,
              uv_process_t* process,
-             const uv_process_options_t options) {
+             const uv_process_options_t* options) {
   int signal_pipe[2] = { -1, -1 };
   int (*pipes)[2];
   int stdio_count;
@@ -354,17 +354,17 @@ int uv_spawn(uv_loop_t* loop,
   int err;
   int i;
 
-  assert(options.file != NULL);
-  assert(!(options.flags & ~(UV_PROCESS_DETACHED |
-                             UV_PROCESS_SETGID |
-                             UV_PROCESS_SETUID |
-                             UV_PROCESS_WINDOWS_HIDE |
-                             UV_PROCESS_WINDOWS_VERBATIM_ARGUMENTS)));
+  assert(options->file != NULL);
+  assert(!(options->flags & ~(UV_PROCESS_DETACHED |
+                              UV_PROCESS_SETGID |
+                              UV_PROCESS_SETUID |
+                              UV_PROCESS_WINDOWS_HIDE |
+                              UV_PROCESS_WINDOWS_VERBATIM_ARGUMENTS)));
 
   uv__handle_init(loop, (uv_handle_t*)process, UV_PROCESS);
   QUEUE_INIT(&process->queue);
 
-  stdio_count = options.stdio_count;
+  stdio_count = options->stdio_count;
   if (stdio_count < 3)
     stdio_count = 3;
 
@@ -378,8 +378,8 @@ int uv_spawn(uv_loop_t* loop,
     pipes[i][1] = -1;
   }
 
-  for (i = 0; i < options.stdio_count; i++) {
-    err = uv__process_init_stdio(options.stdio + i, pipes[i]);
+  for (i = 0; i < options->stdio_count; i++) {
+    err = uv__process_init_stdio(options->stdio + i, pipes[i]);
     if (err)
       goto error;
   }
@@ -442,13 +442,13 @@ int uv_spawn(uv_loop_t* loop,
 
   close(signal_pipe[0]);
 
-  for (i = 0; i < options.stdio_count; i++) {
-    err = uv__process_open_stream(options.stdio + i, pipes[i], i == 0);
+  for (i = 0; i < options->stdio_count; i++) {
+    err = uv__process_open_stream(options->stdio + i, pipes[i], i == 0);
     if (err == 0)
       continue;
 
     while (i--)
-      uv__process_close_stream(options.stdio + i);
+      uv__process_close_stream(options->stdio + i);
 
     goto error;
   }
@@ -457,7 +457,7 @@ int uv_spawn(uv_loop_t* loop,
   QUEUE_INSERT_TAIL(q, &process->queue);
 
   process->pid = pid;
-  process->exit_cb = options.exit_cb;
+  process->exit_cb = options->exit_cb;
   uv__handle_start(process);
 
   free(pipes);

diff --git a/src/win/internal.h b/src/win/internal.h
@@ -311,8 +311,9 @@ void uv_fatal_error(const int errorno, const char* syscall);
 /*
  * Process stdio handles.
  */
-int uv__stdio_create(uv_loop_t* loop, uv_process_options_t* options,
-    BYTE** buffer_ptr);
+int uv__stdio_create(uv_loop_t* loop,
+                     const uv_process_options_t* options,
+                     BYTE** buffer_ptr);
 void uv__stdio_destroy(BYTE* buffer);
 void uv__stdio_noinherit(BYTE* buffer);
 int uv__stdio_verify(BYTE* buffer, WORD size);

diff --git a/src/win/process-stdio.c b/src/win/process-stdio.c
@@ -260,8 +260,9 @@ int uv__create_nul_handle(HANDLE* handle_ptr,
 }
 
 
-int uv__stdio_create(uv_loop_t* loop, uv_process_options_t* options,
-    BYTE** buffer_ptr) {
+int uv__stdio_create(uv_loop_t* loop,
+                     const uv_process_options_t* options,
+                     BYTE** buffer_ptr) {
   BYTE* buffer;
   int count, i;
   int err;

diff --git a/src/win/process.c b/src/win/process.c
@@ -802,8 +802,9 @@ void uv_process_endgame(uv_loop_t* loop, uv_process_t* handle) {
 }
 
 
-int uv_spawn(uv_loop_t* loop, uv_process_t* process,
-    uv_process_options_t options) {
+int uv_spawn(uv_loop_t* loop,
+             uv_process_t* process,
+             const uv_process_options_t* options) {
   int i;
   int err = 0;
   WCHAR* path = NULL;
@@ -814,44 +815,45 @@ int uv_spawn(uv_loop_t* loop, uv_process_t* process,
   PROCESS_INFORMATION info;
   DWORD process_flags;
 
-  if (options.flags & (UV_PROCESS_SETGID | UV_PROCESS_SETUID)) {
+  if (options->flags & (UV_PROCESS_SETGID | UV_PROCESS_SETUID)) {
     return UV_ENOTSUP;
   }
 
-  if (options.file == NULL ||
-      options.args == NULL) {
+  if (options->file == NULL ||
+      options->args == NULL) {
     return UV_EINVAL;
   }
 
-  assert(options.file != NULL);
-  assert(!(options.flags & ~(UV_PROCESS_DETACHED |
-                             UV_PROCESS_SETGID |
-                             UV_PROCESS_SETUID |
-                             UV_PROCESS_WINDOWS_HIDE |
-                             UV_PROCESS_WINDOWS_VERBATIM_ARGUMENTS)));
+  assert(options->file != NULL);
+  assert(!(options->flags & ~(UV_PROCESS_DETACHED |
+                              UV_PROCESS_SETGID |
+                              UV_PROCESS_SETUID |
+                              UV_PROCESS_WINDOWS_HIDE |
+                              UV_PROCESS_WINDOWS_VERBATIM_ARGUMENTS)));
 
   uv_process_init(loop, process);
-  process->exit_cb = options.exit_cb;
+  process->exit_cb = options->exit_cb;
 
-  err = uv_utf8_to_utf16_alloc(options.file, &application);
+  err = uv_utf8_to_utf16_alloc(options->file, &application);
   if (err)
     goto immediate_failure;
 
-  err = make_program_args(options.args,
-                          options.flags & UV_PROCESS_WINDOWS_VERBATIM_ARGUMENTS,
-                          &arguments);
+  err = make_program_args(
+      options->args,
+      options->flags & UV_PROCESS_WINDOWS_VERBATIM_ARGUMENTS,
+      &arguments);
   if (err)
     goto immediate_failure;
 
-  if (options.env) {
-     err = make_program_env(options.env, &env);
+  if (options->env) {
+     err = make_program_env(options->env, &env);
      if (err)
        goto immediate_failure;
   }
 
-  if (options.cwd) {
+  if (options->cwd) {
     /* Explicit cwd */
-    err = uv_utf8_to_utf16_alloc(options.cwd, &cwd);
+    err = uv_utf8_to_utf16_alloc(options->cwd, &cwd);
     if (err)
       goto immediate_failure;
 
@@ -901,7 +903,7 @@ int uv_spawn(uv_loop_t* loop, uv_process_t* process,
     }
   }
 
-  err = uv__stdio_create(loop, &options, &process->child_stdio_buffer);
+  err = uv__stdio_create(loop, options, &process->child_stdio_buffer);
   if (err)
     goto immediate_failure;
 
@@ -929,7 +931,7 @@ int uv_spawn(uv_loop_t* loop, uv_process_t* process,
   startup.hStdOutput = uv__stdio_handle(process->child_stdio_buffer, 1);
   startup.hStdError = uv__stdio_handle(process->child_stdio_buffer, 2);
 
-  if (options.flags & UV_PROCESS_WINDOWS_HIDE) {
+  if (options->flags & UV_PROCESS_WINDOWS_HIDE) {
     /* Use SW_HIDE to avoid any potential process window. */
     startup.wShowWindow = SW_HIDE;
   } else {
@@ -938,7 +940,7 @@ int uv_spawn(uv_loop_t* loop, uv_process_t* process,
 
   process_flags = CREATE_UNICODE_ENVIRONMENT;
 
-  if (options.flags & UV_PROCESS_DETACHED) {
+  if (options->flags & UV_PROCESS_DETACHED) {
     /* Note that we're not setting the CREATE_BREAKAWAY_FROM_JOB flag. That
      * means that libuv might not let you create a fully deamonized process
      * when run under job control. However the type of job control that libuv
@@ -968,7 +970,7 @@ int uv_spawn(uv_loop_t* loop, uv_process_t* process,
 
     /* If the process isn't spawned as detached, assign to the global job */
     /* object so windows will kill it when the parent process dies. */
-    if (!(options.flags & UV_PROCESS_DETACHED)) {
+    if (!(options->flags & UV_PROCESS_DETACHED)) {
       uv_once(&uv_global_job_handle_init_guard_, uv__init_global_job_handle);
 
       if (!AssignProcessToJobObject(uv_global_job_handle_, info.hProcess)) {
@@ -989,8 +991,8 @@ int uv_spawn(uv_loop_t* loop, uv_process_t* process,
     }
 
     /* Set IPC pid to all IPC pipes. */
-    for (i = 0; i < options.stdio_count; i++) {
-      const uv_stdio_container_t* fdopt = &options.stdio[i];
+    for (i = 0; i < options->stdio_count; i++) {
+      const uv_stdio_container_t* fdopt = &options->stdio[i];
       if (fdopt->flags & UV_CREATE_PIPE &&
           fdopt->data.stream->type == UV_NAMED_PIPE &&
           ((uv_pipe_t*) fdopt->data.stream)->ipc) {

diff --git a/test/benchmark-spawn.c b/test/benchmark-spawn.c
@@ -122,7 +122,7 @@ static void spawn(void) {
   options.stdio[1].flags = UV_CREATE_PIPE | UV_WRITABLE_PIPE;
   options.stdio[1].data.stream = (uv_stream_t*)&out;
 
-  r = uv_spawn(loop, &process, options);
+  r = uv_spawn(loop, &process, &options);
   ASSERT(r == 0);
 
   process_open = 1;

diff --git a/test/test-ipc.c b/test/test-ipc.c
@@ -234,7 +234,7 @@ void spawn_helper(uv_pipe_t* channel,
   options.stdio[0].data.stream = (uv_stream_t*)channel;
   options.stdio_count = 1;
 
-  r = uv_spawn(uv_default_loop(), process, options);
+  r = uv_spawn(uv_default_loop(), process, &options);
   ASSERT(r == 0);
 }
 

diff --git a/test/test-ref.c b/test/test-ref.c
@@ -403,7 +403,7 @@ TEST_IMPL(process_ref) {
   options.args = argv;
   options.exit_cb = NULL;
 
-  r = uv_spawn(uv_default_loop(), &h, options);
+  r = uv_spawn(uv_default_loop(), &h, &options);
   ASSERT(r == 0);
 
   uv_unref((uv_handle_t*)&h);