subprocess: change to a fancier API

Introduce mp_subprocess() and related definitions. This is a bit more
flexible than the old stuff. This may or may not be used for a more
complicated feature that involves starting processes, and which would
require more control.

Only port subprocess-posix.c to this API. The player still uses the
"old" API, so for win32 and dummy implementations, the new API is simply
not available, while for POSIX, the old APIs are emulated on top of the
new one. I'm hoping the win32 code can be ported as well, so the ifdefs
in subprocess.c can be dropped, and the player can (if convenient or
needed) use the new API.

1 files changed, 84 insertions, 77 deletions

diff --git a/osdep/subprocess-posix.c b/osdep/subprocess-posix.c
index ff78eb42f8..8165f7f641 100644
--- a/osdep/subprocess-posix.c
+++ b/osdep/subprocess-posix.c
@@ -35,45 +35,34 @@ extern char **environ;
 
 #define SAFE_CLOSE(fd) do { if ((fd) >= 0) close((fd)); (fd) = -1; } while (0)
 
-// A silly helper: automatically skips entries with negative FDs
-static int sparse_poll(struct pollfd *fds, int num_fds, int timeout)
-{
-    struct pollfd p_fds[10];
-    int map[10];
-    if (num_fds > MP_ARRAY_SIZE(p_fds))
-        return -1;
-    int p_num_fds = 0;
-    for (int n = 0; n < num_fds; n++) {
-        map[n] = -1;
-        if (fds[n].fd < 0)
-            continue;
-        map[n] = p_num_fds;
-        p_fds[p_num_fds++] = fds[n];
-    }
-    int r = poll(p_fds, p_num_fds, timeout);
-    for (int n = 0; n < num_fds; n++)
-        fds[n].revents = (map[n] < 0 && r >= 0) ? 0 : p_fds[map[n]].revents;
-    return r;
-}
-
-int mp_subprocess(char **args, struct mp_cancel *cancel, void *ctx,
-                  subprocess_read_cb on_stdout, subprocess_read_cb on_stderr,
-                  char **error)
+void mp_subprocess2(struct mp_subprocess_opts *opts,
+                    struct mp_subprocess_result *res)
 {
     posix_spawn_file_actions_t fa;
     bool fa_destroy = false;
     int status = -1;
-    int p_stdout[2] = {-1, -1};
-    int p_stderr[2] = {-1, -1};
+    int comm_pipe[MP_SUBPROCESS_MAX_FDS][2];
     int devnull = -1;
     pid_t pid = -1;
     bool spawned = false;
     bool killed_by_us = false;
+    int cancel_fd = -1;
 
-    if (on_stdout && mp_make_cloexec_pipe(p_stdout) < 0)
-        goto done;
-    if (on_stderr && mp_make_cloexec_pipe(p_stderr) < 0)
-        goto done;
+    *res = (struct mp_subprocess_result){0};
+
+    for (int n = 0; n < opts->num_fds; n++)
+        comm_pipe[n][0] = comm_pipe[n][1] = -1;
+
+    if (opts->cancel) {
+        cancel_fd = mp_cancel_get_fd(opts->cancel);
+        if (cancel_fd < 0)
+            goto done;
+    }
+
+    for (int n = 0; n < opts->num_fds; n++) {
+        if (opts->fds[n].on_read && mp_make_cloexec_pipe(comm_pipe[n]) < 0)
+            goto done;
+    }
 
     devnull = open("/dev/null", O_RDONLY | O_CLOEXEC);
     if (devnull < 0)
@@ -82,79 +71,97 @@ int mp_subprocess(char **args, struct mp_cancel *cancel, void *ctx,
     if (posix_spawn_file_actions_init(&fa))
         goto done;
     fa_destroy = true;
-    // redirect stdin/stdout/stderr
-    if (posix_spawn_file_actions_adddup2(&fa, devnull, 0))
-        goto done;
-    if (p_stdout[1] >= 0 && posix_spawn_file_actions_adddup2(&fa, p_stdout[1], 1))
-        goto done;
-    if (p_stderr[1] >= 0 && posix_spawn_file_actions_adddup2(&fa, p_stderr[1], 2))
-        goto done;
 
-    if (posix_spawnp(&pid, args[0], &fa, NULL, args, environ)) {
+    // redirect FDs
+    for (int n = 0; n < opts->num_fds; n++) {
+        int src_fd = devnull;
+        if (comm_pipe[n][1] >= 0)
+            src_fd = comm_pipe[n][1];
+        if (opts->fds[n].src_fd >= 0)
+            src_fd = opts->fds[n].src_fd;
+        if (posix_spawn_file_actions_adddup2(&fa, src_fd, opts->fds[n].fd))
+            goto done;
+    }
+
+    char **env = opts->env ? opts->env : environ;
+    if (posix_spawnp(&pid, opts->exe, &fa, NULL, opts->args, env)) {
         pid = -1;
         goto done;
     }
     spawned = true;
 
-    SAFE_CLOSE(p_stdout[1]);
-    SAFE_CLOSE(p_stderr[1]);
+    for (int n = 0; n < opts->num_fds; n++)
+        SAFE_CLOSE(comm_pipe[n][1]);
     SAFE_CLOSE(devnull);
 
-    int *read_fds[2] = {&p_stdout[0], &p_stderr[0]};
-    subprocess_read_cb read_cbs[2] = {on_stdout, on_stderr};
-
-    while (p_stdout[0] >= 0 || p_stderr[0] >= 0) {
-        struct pollfd fds[] = {
-            {.events = POLLIN, .fd = *read_fds[0]},
-            {.events = POLLIN, .fd = *read_fds[1]},
-            {.events = POLLIN, .fd = cancel ? mp_cancel_get_fd(cancel) : -1},
-        };
-        if (sparse_poll(fds, MP_ARRAY_SIZE(fds), -1) < 0 && errno != EINTR)
-            break;
-        for (int n = 0; n < 2; n++) {
-            if (fds[n].revents) {
-                char buf[4096];
-                ssize_t r = read(*read_fds[n], buf, sizeof(buf));
-                if (r < 0 && errno == EINTR)
-                    continue;
-                if (r > 0 && read_cbs[n])
-                    read_cbs[n](ctx, buf, r);
-                if (r <= 0)
-                    SAFE_CLOSE(*read_fds[n]);
+    while (1) {
+        struct pollfd fds[MP_SUBPROCESS_MAX_FDS + 1];
+        int map_fds[MP_SUBPROCESS_MAX_FDS + 1];
+        int num_fds = 0;
+        for (int n = 0; n < opts->num_fds; n++) {
+            if (comm_pipe[n][0] >= 0) {
+                map_fds[num_fds] = n;
+                fds[num_fds++] = (struct pollfd){
+                    .events = POLLIN,
+                    .fd = comm_pipe[n][0],
+                };
             }
         }
-        if (fds[2].revents) {
-            kill(pid, SIGKILL);
-            killed_by_us = true;
+        if (!num_fds)
+            break;
+        if (cancel_fd >= 0) {
+            map_fds[num_fds] = -1;
+            fds[num_fds++] = (struct pollfd){.events = POLLIN, .fd = cancel_fd};
+        }
+
+        if (poll(fds, num_fds, -1) < 0 && errno != EINTR)
             break;
+
+        for (int idx = 0; idx < num_fds; idx++) {
+            if (fds[idx].revents) {
+                int n = map_fds[idx];
+                if (n < 0) {
+                    // cancel_fd
+                    kill(pid, SIGKILL);
+                    killed_by_us = true;
+                    break;
+                } else {
+                    char buf[4096];
+                    ssize_t r = read(comm_pipe[n][0], buf, sizeof(buf));
+                    if (r < 0 && errno == EINTR)
+                        continue;
+                    if (r > 0 && opts->fds[n].on_read)
+                        opts->fds[n].on_read(opts->fds[n].on_read_ctx, buf, r);
+                    if (r <= 0)
+                        SAFE_CLOSE(comm_pipe[n][0]);
+                }
+            }
         }
     }
 
     // Note: it can happen that a child process closes the pipe, but does not
     //       terminate yet. In this case, we would have to run waitpid() in
     //       a separate thread and use pthread_cancel(), or use other weird
-    //       and laborious tricks. So this isn't handled yet.
+    //       and laborious tricks in order to react to mp_cancel.
+    //       So this isn't handled yet.
     while (waitpid(pid, &status, 0) < 0 && errno == EINTR) {}
 
 done:
     if (fa_destroy)
         posix_spawn_file_actions_destroy(&fa);
-    SAFE_CLOSE(p_stdout[0]);
-    SAFE_CLOSE(p_stdout[1]);
-    SAFE_CLOSE(p_stderr[0]);
-    SAFE_CLOSE(p_stderr[1]);
+    for (int n = 0; n < opts->num_fds; n++) {
+        SAFE_CLOSE(comm_pipe[n][0]);
+        SAFE_CLOSE(comm_pipe[n][1]);
+    }
     SAFE_CLOSE(devnull);
 
     if (!spawned || (WIFEXITED(status) && WEXITSTATUS(status) == 127)) {
-        *error = "init";
-        status = -1;
+        res->error = MP_SUBPROCESS_EINIT;
     } else if (WIFEXITED(status)) {
-        *error = NULL;
-        status = WEXITSTATUS(status);
+        res->exit_status = WEXITSTATUS(status);
+    } else if (killed_by_us) {
+        res->error = MP_SUBPROCESS_EKILLED_BY_US;
     } else {
-        *error = "killed";
-        status = killed_by_us ? MP_SUBPROCESS_EKILLED_BY_US : -1;
+        res->error = MP_SUBPROCESS_EGENERIC;
     }
-
-    return status;
 }