Skip to content

Commit

Permalink
fix(Common): fixes for ffProcessAppendOutput
Browse files Browse the repository at this point in the history
- 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.
  • Loading branch information
apocelipes committed Nov 29, 2023
1 parent 968a43c commit 50c2bde
Showing 1 changed file with 32 additions and 24 deletions.
56 changes: 32 additions & 24 deletions src/common/processing_linux.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -20,40 +20,52 @@ 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)
{
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)
{
Expand All @@ -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";
}

0 comments on commit 50c2bde

Please sign in to comment.