Date: Fri, 23 Oct 2009 15:00:15 GMT From: dfilter@FreeBSD.ORG (dfilter service) To: freebsd-bugs@FreeBSD.org Subject: Re: kern/90580: commit references a PR Message-ID: <200910231500.n9NF0FeJ075019@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR kern/90580; it has been noted by GNATS. From: dfilter@FreeBSD.ORG (dfilter service) To: bug-followup@FreeBSD.org Cc: Subject: Re: kern/90580: commit references a PR Date: Fri, 23 Oct 2009 14:50:26 +0000 (UTC) Author: jilles Date: Fri Oct 23 14:50:11 2009 New Revision: 198406 URL: http://svn.freebsd.org/changeset/base/198406 Log: wordexp(3): fix some bugs with signals and long outputs * retry various system calls on EINTR * retry the rest after a short read (common if there is more than about 1K of output) * block SIGCHLD like system(3) does (note that this does not and cannot work fully in threaded programs, they will need to be careful with wait functions) PR: 90580 MFC after: 1 month Modified: head/lib/libc/gen/wordexp.c head/tools/regression/lib/libc/gen/test-wordexp.c Modified: head/lib/libc/gen/wordexp.c ============================================================================== --- head/lib/libc/gen/wordexp.c Fri Oct 23 14:43:17 2009 (r198405) +++ head/lib/libc/gen/wordexp.c Fri Oct 23 14:50:11 2009 (r198406) @@ -28,8 +28,10 @@ #include <sys/cdefs.h> #include <sys/types.h> #include <sys/wait.h> +#include <errno.h> #include <fcntl.h> #include <paths.h> +#include <signal.h> #include <stdio.h> #include <stdlib.h> #include <string.h> @@ -73,6 +75,24 @@ wordexp(const char * __restrict words, w return (0); } +static size_t +we_read_fully(int fd, char *buffer, size_t len) +{ + size_t done; + ssize_t nread; + + done = 0; + do { + nread = _read(fd, buffer + done, len - done); + if (nread == -1 && errno == EINTR) + continue; + if (nread <= 0) + break; + done += nread; + } while (done != len); + return done; +} + /* * we_askshell -- * Use the `wordexp' /bin/sh builtin function to do most of the work @@ -90,20 +110,31 @@ we_askshell(const char *words, wordexp_t size_t sofs; /* Offset into we->we_strings */ size_t vofs; /* Offset into we->we_wordv */ pid_t pid; /* Process ID of child */ + pid_t wpid; /* waitpid return value */ int status; /* Child exit status */ + int error; /* Our return value */ + int serrno; /* errno to return */ char *ifs; /* IFS env. var. */ char *np, *p; /* Handy pointers */ char *nstrings; /* Temporary for realloc() */ char **nwv; /* Temporary for realloc() */ + sigset_t newsigblock, oldsigblock; + serrno = errno; if ((ifs = getenv("IFS")) == NULL) ifs = " \t\n"; if (pipe(pdes) < 0) return (WRDE_NOSPACE); /* XXX */ + (void)sigemptyset(&newsigblock); + (void)sigaddset(&newsigblock, SIGCHLD); + (void)_sigprocmask(SIG_BLOCK, &newsigblock, &oldsigblock); if ((pid = fork()) < 0) { + serrno = errno; _close(pdes[0]); _close(pdes[1]); + (void)_sigprocmask(SIG_SETMASK, &oldsigblock, NULL); + errno = serrno; return (WRDE_NOSPACE); /* XXX */ } else if (pid == 0) { @@ -114,6 +145,7 @@ we_askshell(const char *words, wordexp_t int devnull; char *cmd; + (void)_sigprocmask(SIG_SETMASK, &oldsigblock, NULL); _close(pdes[0]); if (_dup2(pdes[1], STDOUT_FILENO) < 0) _exit(1); @@ -139,10 +171,11 @@ we_askshell(const char *words, wordexp_t * the expanded words separated by nulls. */ _close(pdes[1]); - if (_read(pdes[0], wbuf, 8) != 8 || _read(pdes[0], bbuf, 8) != 8) { - _close(pdes[0]); - _waitpid(pid, &status, 0); - return (flags & WRDE_UNDEF ? WRDE_BADVAL : WRDE_SYNTAX); + if (we_read_fully(pdes[0], wbuf, 8) != 8 || + we_read_fully(pdes[0], bbuf, 8) != 8) { + error = flags & WRDE_UNDEF ? WRDE_BADVAL : WRDE_SYNTAX; + serrno = errno; + goto cleanup; } wbuf[8] = bbuf[8] = '\0'; nwords = strtol(wbuf, NULL, 16); @@ -162,33 +195,38 @@ we_askshell(const char *words, wordexp_t if ((nwv = realloc(we->we_wordv, (we->we_wordc + 1 + (flags & WRDE_DOOFFS ? we->we_offs : 0)) * sizeof(char *))) == NULL) { - _close(pdes[0]); - _waitpid(pid, &status, 0); - return (WRDE_NOSPACE); + error = WRDE_NOSPACE; + goto cleanup; } we->we_wordv = nwv; if ((nstrings = realloc(we->we_strings, we->we_nbytes)) == NULL) { - _close(pdes[0]); - _waitpid(pid, &status, 0); - return (WRDE_NOSPACE); + error = WRDE_NOSPACE; + goto cleanup; } for (i = 0; i < vofs; i++) if (we->we_wordv[i] != NULL) we->we_wordv[i] += nstrings - we->we_strings; we->we_strings = nstrings; - if (_read(pdes[0], we->we_strings + sofs, nbytes) != nbytes) { - _close(pdes[0]); - _waitpid(pid, &status, 0); - return (flags & WRDE_UNDEF ? WRDE_BADVAL : WRDE_SYNTAX); + if (we_read_fully(pdes[0], we->we_strings + sofs, nbytes) != nbytes) { + error = flags & WRDE_UNDEF ? WRDE_BADVAL : WRDE_SYNTAX; + serrno = errno; + goto cleanup; } - if (_waitpid(pid, &status, 0) < 0 || !WIFEXITED(status) || - WEXITSTATUS(status) != 0) { - _close(pdes[0]); - return (flags & WRDE_UNDEF ? WRDE_BADVAL : WRDE_SYNTAX); - } + error = 0; +cleanup: _close(pdes[0]); + do + wpid = _waitpid(pid, &status, 0); + while (wpid < 0 && errno == EINTR); + (void)_sigprocmask(SIG_SETMASK, &oldsigblock, NULL); + if (error != 0) { + errno = serrno; + return (error); + } + if (wpid < 0 || !WIFEXITED(status) || WEXITSTATUS(status) != 0) + return (flags & WRDE_UNDEF ? WRDE_BADVAL : WRDE_SYNTAX); /* * Break the null-terminated expanded word strings out into Modified: head/tools/regression/lib/libc/gen/test-wordexp.c ============================================================================== --- head/tools/regression/lib/libc/gen/test-wordexp.c Fri Oct 23 14:43:17 2009 (r198405) +++ head/tools/regression/lib/libc/gen/test-wordexp.c Fri Oct 23 14:50:11 2009 (r198406) @@ -32,17 +32,36 @@ #include <sys/cdefs.h> __FBSDID("$FreeBSD$"); +#include <sys/wait.h> + #include <assert.h> +#include <errno.h> +#include <signal.h> #include <stdio.h> #include <stdlib.h> #include <string.h> #include <wordexp.h> +static void +chld_handler(int x) +{ + int status, serrno; + + (void)x; + serrno = errno; + while (waitpid(-1, &status, WNOHANG) > 0) + ; + errno = serrno; +} + int main(int argc, char *argv[]) { + struct sigaction sa; wordexp_t we; int r; + int i; + char longdata[6 * 10000 + 1]; /* Test that the macros are there. */ (void)(WRDE_APPEND + WRDE_DOOFFS + WRDE_NOCMD + WRDE_REUSE + @@ -59,6 +78,15 @@ main(int argc, char *argv[]) assert(we.we_wordv[2] == NULL); wordfree(&we); + /* Long output. */ + for (i = 0; i < 10000; i++) + snprintf(longdata + 6 * i, 7, "%05d ", i); + r = wordexp(longdata, &we, 0); + assert(r == 0); + assert(we.we_wordc == 10000); + assert(we.we_wordv[10000] == NULL); + wordfree(&we); + /* WRDE_DOOFFS */ we.we_offs = 3; r = wordexp("hello world", &we, WRDE_DOOFFS); @@ -167,6 +195,20 @@ main(int argc, char *argv[]) r = wordexp("test } test", &we, 0); assert(r == WRDE_BADCHAR); + /* With a SIGCHLD handler that reaps all zombies. */ + sa.sa_flags = 0; + sigemptyset(&sa.sa_mask); + sa.sa_handler = chld_handler; + r = sigaction(SIGCHLD, &sa, NULL); + assert(r == 0); + r = wordexp("hello world", &we, 0); + assert(r == 0); + assert(we.we_wordc == 2); + assert(strcmp(we.we_wordv[0], "hello") == 0); + assert(strcmp(we.we_wordv[1], "world") == 0); + assert(we.we_wordv[2] == NULL); + wordfree(&we); + printf("PASS wordexp()\n"); printf("PASS wordfree()\n"); _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200910231500.n9NF0FeJ075019>