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

diff --git a/archival/libunarchive/open_transformer.c b/archival/libunarchive/open_transformer.c
index 0ee080621a054c2e0221028cf102f4c497bbe20f..93f01be6f21e8118a54f3e8abeef9d7f643a7f0f 100644 (file)
--- 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 9bf9058d885614e2d3e0252be42e0cd3d1376218..f0d3971140495fa18e12a12cfa0b55cc526660ce 100644 (file)
--- 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 6c6b4863c8985b5f847e4530054884897cca918d..cf00b52505c01b019cf2d8041c965030b3d2e446 100644 (file)
--- 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)