From 50c2bded056dc185736db5b7a9bb444dd7e30823 Mon Sep 17 00:00:00 2001
From: apocelipes <seve3r@outlook.com>
Date: Wed, 29 Nov 2023 19:28:28 +0900
Subject: [PATCH] fix(Common): fixes for ffProcessAppendOutput

- Use pipe2 on Linux and BSDs, which can set all flags through a single syscall.
- macOS/darwin does not support pipe2, add a wrapper for it.
- Close pipes when fork() failed. These fds should be closed when
  the function returns.
- Set O_CLOEXEC at pipes, the exec() family of functions will close
  this fds automatically.
- Use _exit instead of exit. Exit calls the callbacks registered by atexit
  and flushes the stdio's buffer, which can lead to some unexpected behavior,
  especially after an exec syscall failure.
- Fix timeout.
- Not use non-blocking IO. because we only wait for one file, and cannot
  do other things during waiting.
---
 src/common/processing_linux.c | 56 ++++++++++++++++++++---------------
 1 file changed, 32 insertions(+), 24 deletions(-)

diff --git a/src/common/processing_linux.c b/src/common/processing_linux.c
index 4c136df5f8..35c461bbc8 100644
--- a/src/common/processing_linux.c
+++ b/src/common/processing_linux.c
@@ -11,7 +11,7 @@
 #include <errno.h>
 #include <sys/wait.h>
 
-enum { FF_PIPE_BUFSIZ = 4096 };
+#define FF_PIPE_BUFSIZ 16384
 
 static inline void waitpid_wrapper(const pid_t* pid)
 {
@@ -20,16 +20,33 @@ static inline void waitpid_wrapper(const pid_t* pid)
         waitpid(*pid, NULL, 0);
 }
 
+static inline int ffPipe2(int *fds, int flags) {
+    #ifdef __APPLE__
+        if(pipe(fds) == -1)
+            return -1;
+        fcntl(fds[0], F_SETFL, fcntl(fds[0], F_GETFL) | flags);
+        fcntl(fds[1], F_SETFL, fcntl(fds[1], F_GETFL) | flags);
+        return 0;
+    #else
+        return pipe2(fds, flags);
+    #endif
+}
+
 const char* ffProcessAppendOutput(FFstrbuf* buffer, char* const argv[], bool useStdErr)
 {
     int pipes[2];
+    const int timeout = instance.config.general.processingTimeout;
+    const int pipeFlags = O_CLOEXEC;
 
-    if(pipe(pipes) == -1)
+    if(ffPipe2(pipes, pipeFlags) == -1)
         return "pipe() failed";
 
     __attribute__((__cleanup__(waitpid_wrapper))) pid_t childPid = fork();
-    if(childPid == -1)
+    if(childPid == -1) {
+        close(pipes[0]);
+        close(pipes[1]);
         return "fork() failed";
+    }
 
     //Child
     if(childPid == 0)
@@ -37,23 +54,18 @@ const char* ffProcessAppendOutput(FFstrbuf* buffer, char* const argv[], bool use
         int nullFile = open("/dev/null", O_WRONLY);
         dup2(pipes[1], useStdErr ? STDERR_FILENO : STDOUT_FILENO);
         dup2(nullFile, useStdErr ? STDOUT_FILENO : STDERR_FILENO);
-        close(pipes[0]);
-        close(pipes[1]);
         setenv("LANG", "C", 1);
         execvp(argv[0], argv);
-        exit(901);
+        _exit(127);
     }
 
     //Parent
     close(pipes[1]);
 
     int FF_AUTO_CLOSE_FD childPipeFd = pipes[0];
+    char str[FF_PIPE_BUFSIZ];
 
-    int timeout = instance.config.general.processingTimeout;
-    if (timeout >= 0)
-        fcntl(childPipeFd, F_SETFL, fcntl(childPipeFd, F_GETFL) | O_NONBLOCK);
-
-    do
+    while(1)
     {
         if (timeout >= 0)
         {
@@ -70,18 +82,14 @@ const char* ffProcessAppendOutput(FFstrbuf* buffer, char* const argv[], bool use
             }
         }
 
-        char str[FF_PIPE_BUFSIZ];
-        while (true)
-        {
-            ssize_t nRead = read(childPipeFd, str, FF_PIPE_BUFSIZ);
-            if (nRead > 0)
-                ffStrbufAppendNS(buffer, (uint32_t) nRead, str);
-            else if (nRead == 0)
-                return NULL;
-            else if (nRead < 0)
-                break;
-        }
-    } while (errno == EAGAIN);
+        ssize_t nRead = read(childPipeFd, str, FF_PIPE_BUFSIZ);
+        if (nRead > 0)
+            ffStrbufAppendNS(buffer, (uint32_t) nRead, str);
+        else if (nRead == 0)
+            return NULL;
+        else if (nRead < 0)
+            break;
+    };
 
-    return NULL;
+    return "read(childPipeFd, str, FF_PIPE_BUFSIZ) failed";
 }