Date: Tue, 23 Apr 1996 22:31:20 +1000 From: Bruce Evans <bde@zeta.org.au> To: freebsd-hackers@freebsd.org, jraynard@dial.pipex.com Subject: Re: Flaws in system() implementation? Message-ID: <199604231231.WAA20624@godzilla.zeta.org.au>
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:- The POSIX.2 draft has a lot to say about system(). It even gives a complete sample of an implementation. (The draft standard for system() is one of 2 or 3 sections of POSIX.2 that I've seen. Don't ask...) >1. Use of a wait union when an int would do. Good. >2. Use of the soon-to-be-obsolescent vfork() instead of fork(). Reports of its death seem to be premature :-). >3. Returns 0 if fork() fails, when -1 seems more appropriate. It actually returns (127 << 8): >! pstat.w_status = 0; >! pstat.w_retcode = 127; >! return(pstat.w_status); as if the fork had succeeded and the exec had failed. The POSIX sample implementation returns -1. This is better than (127 << 8). The FreeBSD manpage for system() mixes exit statuses with return values in a confusing way. system() never _returns_ 127, because there is no signal 127... >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(). This is most important. More points: 5. Use of the crufty signal() and deprecated sigblock() and sigsetmask() instead of POSIX signal handling interfaces. It isn't clear whether the sa_mask and sa_flags used by signal() give the correct behaviour. 6. Missing EINTR handling for waitpid(). This might be OK if SA_RESTART was forced, but see point 5 - it is now clear that the sa_flags used by signal() don't give the correct behaviour if the application has used siginterrupt() to change the default of SA_RESTART. >Here's an untested patch which addresses these points. >Comments/corrections welcomed! It seems OK. Some of these points also apply to popen/pclose, but the FreeBSD already seems to be correct although unnecessarily unportable. E.g., it handles EINTR. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199604231231.WAA20624>