Date: Mon, 22 Apr 1996 21:42:43 GMT From: James Raynard <jraynard@dial.pipex.com> To: freebsd-hackers@freebsd.org Subject: Flaws in system() implementation? Message-ID: <199604222142.VAA00923@dial.pipex.com>
next 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.
2. Use of the soon-to-be-obsolescent vfork() instead of fork().
3. Returns 0 if fork() fails, when -1 seems more appropriate.
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
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?199604222142.VAA00923>