7e0fbf9c2
Denis Vlasenko
2007-09-04
Subject
tar: conditionally don't wait for vforked child to exec, as it always works right on Linux, and anyway mayresult only on less-than-clear error message only, it will not cause tar to misbehave.
Binary
No binary (build failed)
Architecture & Compiler Flags
—
Build Log
(log yok)
Commit Diff GitHub →
archival/libunarchive/open_transformer.c+4 -2
| @@ -25,8 +25,10 @@ int open_transformer(int src_fd, | |
| close(fd_pipe[0]); /* We don't wan't to read from the parent */ | |
| // FIXME: error check? | |
| transformer(src_fd, fd_pipe[1]); | |
| - | close(fd_pipe[1]); /* Send EOF */ |
| - | close(src_fd); |
| + | if (ENABLE_FEATURE_CLEAN_UP) { |
| + | close(fd_pipe[1]); /* Send EOF */ |
| + | close(src_fd); |
| + | } |
| exit(0); | |
| /* notreached */ | |
| } | |
archival/tar.c+47 -35
| @@ -106,7 +106,7 @@ enum TarFileType { | |
| typedef enum TarFileType TarFileType; | |
| /* Might be faster (and bigger) if the dev/ino were stored in numeric order;) */ | |
| - | static void addHardLinkInfo(HardLinkInfo ** hlInfoHeadPtr, |
| + | static void addHardLinkInfo(HardLinkInfo **hlInfoHeadPtr, |
| struct stat *statbuf, | |
| const char *fileName) | |
| { | |
| @@ -122,7 +122,7 @@ static void addHardLinkInfo(HardLinkInfo ** hlInfoHeadPtr, | |
| strcpy(hlInfo->name, fileName); | |
| } | |
| - | static void freeHardLinkInfo(HardLinkInfo ** hlInfoHeadPtr) |
| + | static void freeHardLinkInfo(HardLinkInfo **hlInfoHeadPtr) |
| { | |
| HardLinkInfo *hlInfo; | |
| HardLinkInfo *hlInfoNext; | |
| @@ -139,7 +139,7 @@ static void freeHardLinkInfo(HardLinkInfo ** hlInfoHeadPtr) | |
| } | |
| /* Might be faster (and bigger) if the dev/ino were stored in numeric order;) */ | |
| - | static HardLinkInfo *findHardLinkInfo(HardLinkInfo * hlInfo, struct stat *statbuf) |
| + | static HardLinkInfo *findHardLinkInfo(HardLinkInfo *hlInfo, struct stat *statbuf) |
| { | |
| while (hlInfo) { | |
| if ((statbuf->st_ino == hlInfo->ino) && (statbuf->st_dev == hlInfo->dev)) | |
| @@ -504,13 +504,21 @@ static int writeTarFile(const int tar_fd, const int verboseFlag, | |
| bb_perror_msg_and_die("cannot stat tar file"); | |
| if ((ENABLE_FEATURE_TAR_GZIP || ENABLE_FEATURE_TAR_BZIP2) && gzip) { | |
| - | int gzipDataPipe[2] = { -1, -1 }; |
| - | int gzipStatusPipe[2] = { -1, -1 }; |
| + | // On Linux, vfork never unpauses parent early, although standard |
| + | // allows for that. Do we want to waste bytes checking for it? |
| + | #define WAIT_FOR_CHILD 0 |
| + | |
| volatile int vfork_exec_errno = 0; | |
| + | #if WAIT_FOR_CHILD |
| + | struct { int rd; int wr; } gzipStatusPipe; |
| + | #endif |
| + | struct { int rd; int wr; } gzipDataPipe; |
| const char *zip_exec = (gzip == 1) ? "gzip" : "bzip2"; | |
| - | xpipe(gzipDataPipe); |
| - | xpipe(gzipStatusPipe); |
| + | xpipe(&gzipDataPipe.rd); |
| + | #if WAIT_FOR_CHILD |
| + | xpipe(&gzipStatusPipe.rd); |
| + | #endif |
| signal(SIGPIPE, SIG_IGN); /* we only want EPIPE on errors */ | |
| @@ -522,41 +530,45 @@ static int writeTarFile(const int tar_fd, const int verboseFlag, | |
| #endif | |
| gzipPid = vfork(); | |
| + | if (gzipPid < 0) |
| + | bb_perror_msg_and_die("vfork gzip"); |
| if (gzipPid == 0) { | |
| - | dup2(gzipDataPipe[0], 0); |
| - | close(gzipDataPipe[1]); |
| - | |
| - | dup2(tbInfo.tarFd, 1); |
| - | |
| - | close(gzipStatusPipe[0]); |
| - | fcntl(gzipStatusPipe[1], F_SETFD, FD_CLOEXEC); /* close on exec shows success */ |
| - | |
| + | /* child */ |
| + | xmove_fd(tbInfo.tarFd, 1); |
| + | xmove_fd(gzipDataPipe.rd, 0); |
| + | close(gzipDataPipe.wr); |
| + | #if WAIT_FOR_CHILD |
| + | close(gzipStatusPipe.rd); |
| + | fcntl(gzipStatusPipe.wr, F_SETFD, FD_CLOEXEC); |
| + | #endif |
| + | /* exec gzip/bzip2 program/applet */ |
| BB_EXECLP(zip_exec, zip_exec, "-f", NULL); | |
| vfork_exec_errno = errno; | |
| + | _exit(1); |
| + | } |
| - | close(gzipStatusPipe[1]); |
| - | exit(-1); |
| - | } else if (gzipPid > 0) { |
| - | close(gzipDataPipe[0]); |
| - | close(gzipStatusPipe[1]); |
| - | |
| - | while (1) { |
| - | char buf; |
| - | |
| - | int n = full_read(gzipStatusPipe[0], &buf, 1); |
| + | /* parent */ |
| + | xmove_fd(gzipDataPipe.wr, tbInfo.tarFd); |
| + | close(gzipDataPipe.rd); |
| + | #if WAIT_FOR_CHILD |
| + | close(gzipStatusPipe.wr); |
| + | while (1) { |
| + | char buf; |
| + | int n; |
| - | if (n == 0 && vfork_exec_errno != 0) { |
| - | errno = vfork_exec_errno; |
| - | bb_perror_msg_and_die("cannot exec %s", zip_exec); |
| - | } else if ((n < 0) && (errno == EAGAIN || errno == EINTR)) |
| - | continue; /* try it again */ |
| - | break; |
| - | } |
| - | close(gzipStatusPipe[0]); |
| + | /* Wait until child execs (or fails to) */ |
| + | n = full_read(gzipStatusPipe.rd, &buf, 1); |
| + | if ((n < 0) && (/*errno == EAGAIN ||*/ errno == EINTR)) |
| + | continue; /* try it again */ |
| - | tbInfo.tarFd = gzipDataPipe[1]; |
| - | } else bb_perror_msg_and_die("vfork gzip"); |
| + | } |
| + | close(gzipStatusPipe.rd); |
| + | #endif |
| + | if (vfork_exec_errno) { |
| + | errno = vfork_exec_errno; |
| + | bb_perror_msg_and_die("cannot exec %s", zip_exec); |
| + | } |
| } | |
| tbInfo.excludeList = exclude; | |
include/libbb.h+3 -0
| @@ -512,6 +512,9 @@ int execable_file(const char *name); | |
| char *find_execable(const char *filename); | |
| int exists_execable(const char *filename); | |
| + | /* BB_EXECxx always execs (it's not doing NOFORK/NOEXEC stuff), |
| + | * but it may exec busybox and call applet instead of searching PATH. |
| + | */ |
| #if ENABLE_FEATURE_PREFER_APPLETS | |
| int bb_execvp(const char *file, char *const argv[]); | |
| #define BB_EXECVP(prog,cmd) bb_execvp(prog,cmd) | |