From 7e0fbf9c26350a819661241bc925cb88f26bb992 Mon Sep 17 00:00:00 2001 From: Denis Vlasenko Date: Tue, 4 Sep 2007 19:33:22 +0000 Subject: [PATCH] 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. function old new delta open_transformer 98 80 -18 writeTarFile 714 547 -167 ------------------------------------------------------------------------------ (add/remove: 0/0 grow/shrink: 0/2 up/down: 0/-185) Total: -185 bytes text data bss dec hex filename 770651 1051 10764 782466 bf082 busybox_old 770463 1051 10764 782278 befc6 busybox_unstripped --- archival/libunarchive/open_transformer.c | 6 +- archival/tar.c | 82 ++++++++++++++---------- include/libbb.h | 3 + 3 files changed, 54 insertions(+), 37 deletions(-) diff --git a/archival/libunarchive/open_transformer.c b/archival/libunarchive/open_transformer.c index 0ee080621..93f01be6f 100644 --- a/archival/libunarchive/open_transformer.c +++ b/archival/libunarchive/open_transformer.c @@ -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 */ } diff --git a/archival/tar.c b/archival/tar.c index 9bf9058d8..f0d397114 100644 --- a/archival/tar.c +++ b/archival/tar.c @@ -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; diff --git a/include/libbb.h b/include/libbb.h index 6c6b4863c..cf00b5250 100644 --- a/include/libbb.h +++ b/include/libbb.h @@ -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) -- 2.25.1