More spawned process handling updates.

Document ws_pipe.h. Define invalid PIDs in one place.

Extcap didn't use stdin before 1a0987904f. Make sure we close it.

Change-Id: I7a69cd9b5137ae82435e64628a22e4d812d58f89
Reviewed-on: https://code.wireshark.org/review/26226
Petri-Dish: Gerald Combs <gerald@wireshark.org>
Tested-by: Petri Dish Buildbot
Reviewed-by: Gerald Combs <gerald@wireshark.org>

capture_opts.c

@@ -55,7 +55,7 @@ capture_opts_init(capture_options *capture_opts)
     capture_opts->default_options.extcap_fifo     = NULL;
     capture_opts->default_options.extcap_args     = NULL;
     capture_opts->default_options.extcap_pipedata = NULL;
-    capture_opts->default_options.extcap_pid      = INVALID_EXTCAP_PID;
+    capture_opts->default_options.extcap_pid      = WS_INVALID_PID;
 #ifdef _WIN32
     capture_opts->default_options.extcap_pipe_h   = INVALID_HANDLE_VALUE;
     capture_opts->default_options.extcap_control_in_h  = INVALID_HANDLE_VALUE;
@@ -684,7 +684,7 @@ capture_opts_add_iface_opt(capture_options *capture_opts, const char *optarg_str
     interface_opts.promisc_mode = capture_opts->default_options.promisc_mode;
     interface_opts.extcap_fifo = g_strdup(capture_opts->default_options.extcap_fifo);
     interface_opts.extcap_args = NULL;
-    interface_opts.extcap_pid = INVALID_EXTCAP_PID;
+    interface_opts.extcap_pid = WS_INVALID_PID;
     interface_opts.extcap_pipedata = NULL;
 #ifdef _WIN32
     interface_opts.extcap_pipe_h = INVALID_HANDLE_VALUE;
@@ -1126,7 +1126,7 @@ capture_opts_del_iface(capture_options *capture_opts, guint if_index)
     g_free(interface_opts->extcap_fifo);
     if (interface_opts->extcap_args)
         g_hash_table_unref(interface_opts->extcap_args);
-    if (interface_opts->extcap_pid != INVALID_EXTCAP_PID)
+    if (interface_opts->extcap_pid != WS_INVALID_PID)
         g_spawn_close_pid(interface_opts->extcap_pid);
     g_free(interface_opts->extcap_pipedata);
     g_free(interface_opts->extcap_control_in);
@@ -1177,7 +1177,7 @@ collect_ifaces(capture_options *capture_opts)
             interface_opts.extcap_fifo = NULL;
             interface_opts.extcap_pipedata = NULL;
             interface_opts.extcap_args = device->external_cap_args_settings;
-            interface_opts.extcap_pid = INVALID_EXTCAP_PID;
+            interface_opts.extcap_pid = WS_INVALID_PID;
             if (interface_opts.extcap_args)
                 g_hash_table_ref(interface_opts.extcap_args);
             interface_opts.extcap_pipedata = NULL;

capture_opts.h

@@ -211,7 +211,7 @@ typedef struct interface_options_tag {
     gchar            *extcap;
     gchar            *extcap_fifo;
     GHashTable       *extcap_args;
-    GPid              extcap_pid;           /* pid of running process or INVALID_EXTCAP_PID */
+    GPid              extcap_pid;           /* pid of running process or WS_INVALID_PID */
     gpointer          extcap_pipedata;
     guint             extcap_child_watch;
 #ifdef _WIN32

extcap.c

@@ -1089,13 +1089,13 @@ void extcap_if_cleanup(capture_options *capture_opts, gchar **errormsg)
             interface_opts->extcap_child_watch = 0;
         }
 
-        if (interface_opts->extcap_pid != INVALID_EXTCAP_PID)
+        if (interface_opts->extcap_pid != WS_INVALID_PID)
         {
 #ifdef _WIN32
             TerminateProcess(interface_opts->extcap_pid, 0);
 #endif
             g_spawn_close_pid(interface_opts->extcap_pid);
-            interface_opts->extcap_pid = INVALID_EXTCAP_PID;
+            interface_opts->extcap_pid = WS_INVALID_PID;
 
             g_free(interface_opts->extcap_pipedata);
             interface_opts->extcap_pipedata = NULL;
@@ -1147,7 +1147,7 @@ void extcap_child_watch_cb(GPid pid, gint status, gpointer user_data)
             pipedata = (ws_pipe_t *)interface_opts->extcap_pipedata;
             if (pipedata != NULL)
             {
-                interface_opts->extcap_pid = INVALID_EXTCAP_PID;
+                interface_opts->extcap_pid = WS_INVALID_PID;
                 pipedata->exitcode = 0;
 #ifndef _WIN32
                 if (WIFEXITED(status))
@@ -1292,7 +1292,7 @@ extcap_init_interfaces(capture_options *capture_opts)
     for (i = 0; i < capture_opts->ifaces->len; i++)
     {
         GPtrArray *args = NULL;
-        GPid pid = INVALID_EXTCAP_PID;
+        GPid pid = WS_INVALID_PID;
 
         interface_opts = &g_array_index(capture_opts->ifaces, interface_options, i);
 
@@ -1337,12 +1337,13 @@ extcap_init_interfaces(capture_options *capture_opts)
         g_ptr_array_foreach(args, (GFunc)g_free, NULL);
         g_ptr_array_free(args, TRUE);
 
-        if (pid == INVALID_EXTCAP_PID)
+        if (pid == WS_INVALID_PID)
         {
             g_free(pipedata);
             continue;
         }
 
+        ws_close(pipedata->stdin_fd);
         interface_opts->extcap_pid = pid;
 
         interface_opts->extcap_child_watch =
@@ -1356,7 +1357,7 @@ extcap_init_interfaces(capture_options *capture_opts)
          * Wait on multiple object in case of extcap termination
          * without opening pipe.
          */
-        if (pid != INVALID_EXTCAP_PID)
+        if (pid != WS_INVALID_PID)
         {
             HANDLE pipe_handles[3];
             int num_pipe_handles = 1;

wsutil/ws_pipe.c

@@ -170,9 +170,16 @@ gboolean ws_pipe_spawn_sync(gchar *dirname, gchar *command, gint argc, gchar **a
     return result;
 }
 
+void ws_pipe_init(ws_pipe_t *ws_pipe)
+{
+    if (!ws_pipe) return;
+    memset(ws_pipe, 0, sizeof(ws_pipe_t));
+    ws_pipe->pid = WS_INVALID_PID;
+}
+
 GPid ws_pipe_spawn_async(ws_pipe_t *ws_pipe, GPtrArray *args)
 {
-    GPid pid = INVALID_EXTCAP_PID;
+    GPid pid = WS_INVALID_PID;
 
 #ifdef _WIN32
     gint cnt = 0;
@@ -256,9 +263,14 @@ GPid ws_pipe_spawn_async(ws_pipe_t *ws_pipe, GPtrArray *args)
 
     g_setenv("PATH", oldpath, TRUE);
 #else
-    g_spawn_async_with_pipes(NULL, (gchar **)args->pdata, NULL,
+    GError *error = NULL;
+    gboolean spawned = g_spawn_async_with_pipes(NULL, (gchar **)args->pdata, NULL,
                              (GSpawnFlags) G_SPAWN_DO_NOT_REAP_CHILD, NULL, NULL,
-                             &pid, &ws_pipe->stdin_fd, &ws_pipe->stdout_fd, &ws_pipe->stderr_fd, NULL);
+                             &pid, &ws_pipe->stdin_fd, &ws_pipe->stdout_fd, &ws_pipe->stderr_fd, &error);
+    if (!spawned) {
+        g_log(LOG_DOMAIN_CAPTURE, G_LOG_LEVEL_DEBUG, "Error creating async pipe: %s", error->message);
+        g_free(error->message);
+    }
 #endif
 
     ws_pipe->pid = pid;

wsutil/ws_pipe.h

@@ -12,16 +12,11 @@
 #ifndef __WS_PIPE_H__
 #define __WS_PIPE_H__
 
-#include "ws_symbol_export.h"
+// ws_symbol_export and WS_INVALID_PID
+#include "wsutil/processes.h"
 
 #include <glib.h>
 
-#ifdef _WIN32
-#define INVALID_EXTCAP_PID INVALID_HANDLE_VALUE
-#else
-#define INVALID_EXTCAP_PID (GPid)-1
-#endif
-
 #ifdef _WIN32
 #include <windows.h>
 #include <io.h>
@@ -44,16 +39,57 @@ typedef struct _ws_pipe_t {
 #endif
 } ws_pipe_t;
 
+/**
+ * @brief Run a process using g_spawn_sync on UNIX and Linux, and
+ *        CreateProcess on Windows. Wait for it to finish.
+ * @param [IN] dirname Initial working directory.
+ * @param [IN] command Command to run.
+ * @param [IN] argc Number of arguments for the command, not including the command itself.
+ * @param [IN] argv Arguments for the command, not including the command itself.
+ * @param [OUT] command_output If not NULL, receives a copy of the command output. Must be g_freed.
+ * @return TRUE on success or FALSE on failure.
+ */
 WS_DLL_PUBLIC gboolean ws_pipe_spawn_sync ( gchar * dirname, gchar * command, gint argc, gchar ** argv, gchar ** command_output );
 
+/**
+ * @brief Initialize a ws_pipe_t struct. Sets .pid to WS_INVALID_PID and all other members to 0 or NULL.
+ * @param ws_pipe [IN] The pipe to initialize.
+ */
+WS_DLL_PUBLIC void ws_pipe_init(ws_pipe_t *ws_pipe);
+
+/**
+ * @brief Start a process using g_spawn_sync on UNIX and Linux, and CreateProcess on Windows.
+ * @param ws_pipe The process PID, stdio descriptors, etc.
+ * @param args The command to run along with its arguments.
+ * @return A valid PID on success, otherwise WS_INVALID_PID.
+ */
 WS_DLL_PUBLIC GPid ws_pipe_spawn_async (ws_pipe_t * ws_pipe, GPtrArray * args );
 
+/**
+ * @brief Wait for a set of handles using WaitForMultipleObjects. Windows only.
+ * @param pipe_handles An array of handles
+ * @param num_pipe_handles The size of the array.
+ * @param pid Child process PID.
+ * @return TRUE on success or FALSE on failure.
+ */
 #ifdef _WIN32
 WS_DLL_PUBLIC gboolean ws_pipe_wait_for_pipe(HANDLE * pipe_handles, int num_pipe_handles, HANDLE pid);
 #endif
 
+/**
+ * @brief Check to see if a file descriptor has data available.
+ * @param pipe_fd File descriptor, usually ws_pipe_t .stdout_fd or .stderr_fd.
+ * @return TRUE if data is available or FALSE otherwise.
+ */
 WS_DLL_PUBLIC gboolean ws_pipe_data_available(int pipe_fd);
 
+/**
+ * @brief Read up to buffer_size - 1 bytes from a pipe and append '\0' to the buffer.
+ * @param read_pipe File descriptor, usually ws_pipe_t .stdout_fd or .stderr_fd.
+ * @param buffer String buffer.
+ * @param buffer_size String buffer size.
+ * @return TRUE if zero or more bytes were read without error, FALSE otherwise.
+ */
 WS_DLL_PUBLIC gboolean ws_read_string_from_pipe(ws_pipe_handle read_pipe,
     gchar *buffer, size_t buffer_size);