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>