Date: Mon, 22 Apr 1996 15:44:24 -0700 (PDT) From: "JULIAN Elischer" <julian@ref.tfs.com> To: jraynard@dial.pipex.com (James Raynard) Cc: hackers@freebsd.org Subject: Re: Flaws in system() implementation? Message-ID: <199604222244.PAA20222@ref.tfs.com> In-Reply-To: <199604222142.VAA00923@dial.pipex.com> from "James Raynard" at Apr 22, 96 09:42:43 pm
next in thread | previous in thread | raw e-mail | index | archive | help
> > > Inspired by Rich Stevens' implementation of system() in his book > "Advanced Programming in the Unix Environment", I've been looking at > the FreeBSD implementation and was struck by the following points:- > > 1. Use of a wait union when an int would do. This is the correct POSIX thing to do.. check the man page for wait. > > > 3. Returns 0 if fork() fails, when -1 seems more appropriate. POSIX says: If "command" is a null pointer, the system() function returns non-zero only if a command processor is available. If "command" is a nin null pointer the system() function returnsa the termination status of the command language interpretter in the format specified by the waitpid() function. [...] If a child process cannot be created, or if the termination status of hte command interpretter cannot be obtained system() returns -1 and sets errno to indicate the error. > > 4. The SIGINT and SIGQUIT are not ignored until after the fork(). > If the child runs first, one of these signals could in theory be the child MUST run first in vfork() > generated before the parent gets around to ignoring it. Hence the > dispositions should be changed before the fork(). > > Here's an untested patch which addresses these points. > Comments/corrections welcomed! > > (In /usr/src/lib/libc/stdlib) > *** system.c~ Mon Apr 22 21:04:59 1996 > --- system.c Mon Apr 22 21:20:18 1996 > *************** > *** 46,76 **** > system(command) > const char *command; > { > - union wait pstat; > pid_t pid; > ! int omask; > sig_t intsave, quitsave; > > if (!command) /* just checking... */ > return(1); > > omask = sigblock(sigmask(SIGCHLD)); > ! switch(pid = vfork()) { > case -1: /* error */ > ! (void)sigsetmask(omask); > ! pstat.w_status = 0; > ! pstat.w_retcode = 127; > ! return(pstat.w_status); > case 0: /* child */ > (void)sigsetmask(omask); > execl(_PATH_BSHELL, "sh", "-c", command, (char *)NULL); > _exit(127); > } > - intsave = signal(SIGINT, SIG_IGN); > - quitsave = signal(SIGQUIT, SIG_IGN); > - pid = waitpid(pid, (int *)&pstat, 0); > (void)sigsetmask(omask); > (void)signal(SIGINT, intsave); > (void)signal(SIGQUIT, quitsave); > ! return(pid == -1 ? -1 : pstat.w_status); > } > --- 46,77 ---- > system(command) > const char *command; > { > pid_t pid; > ! int omask, pstat; > sig_t intsave, quitsave; > > if (!command) /* just checking... */ > return(1); > > + intsave = signal(SIGINT, SIG_IGN); > + quitsave = signal(SIGQUIT, SIG_IGN); > omask = sigblock(sigmask(SIGCHLD)); > ! switch(pid = fork()) { > case -1: /* error */ > ! break; > case 0: /* child */ > (void)sigsetmask(omask); > + (void)signal(SIGINT, intsave); > + (void)signal(SIGQUIT, quitsave); > execl(_PATH_BSHELL, "sh", "-c", command, (char *)NULL); > _exit(127); > + default: /* parent */ > + pid = waitpid(pid, &pstat, 0); > + break; > } > (void)sigsetmask(omask); > (void)signal(SIGINT, intsave); > (void)signal(SIGQUIT, quitsave); > ! return(pid == -1 ? -1 : pstat); > } >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199604222244.PAA20222>