Re: [PATCH] libc: make system() block SIGCHLD

2012-01-16 Thread Richard Braun
On Sun, Jan 15, 2012 at 06:51:15PM -0500, Mike Frysinger wrote: On Sunday 15 January 2012 14:36:55 Richard Braun wrote: On Sun, Jan 15, 2012 at 10:04:58AM -0500, Rich Felker wrote: Your report is wrong. system is REQUIRED by POSIX to change the signal disposition for SIGCHLD, not just to

Re: [PATCH] libc: make system() block SIGCHLD

2012-01-16 Thread Mike Frysinger
On Monday 16 January 2012 04:00:08 Richard Braun wrote: On Sun, Jan 15, 2012 at 06:51:15PM -0500, Mike Frysinger wrote: On Sunday 15 January 2012 14:36:55 Richard Braun wrote: On Sun, Jan 15, 2012 at 10:04:58AM -0500, Rich Felker wrote: Your report is wrong. system is REQUIRED by POSIX

[PATCH] libc: make system() block SIGCHLD

2012-01-15 Thread Richard Braun
When built without NPTL support (or for a sparc target), the system() function doesn't conform to its specification. Namely, it resets the SIGCHLD handler to its default instead of blocking the signal, which may result in lost signals if a custom handler was installed. Replace this by appropriate

Re: [PATCH] libc: make system() block SIGCHLD

2012-01-15 Thread Rich Felker
On Sun, Jan 15, 2012 at 12:46:33PM +0100, Richard Braun wrote: When built without NPTL support (or for a sparc target), the system() function doesn't conform to its specification. Namely, it resets the SIGCHLD handler to its default instead of blocking the signal, which may result in lost

Re: [PATCH] libc: make system() block SIGCHLD

2012-01-15 Thread Richard Braun
On Sun, Jan 15, 2012 at 10:04:58AM -0500, Rich Felker wrote: Your report is wrong. system is REQUIRED by POSIX to change the signal disposition for SIGCHLD, not just to block the signal. It should change it to SIG_IGN, not SIG_DFL, but for practical purposes these are the same or similar. I

Re: [PATCH] libc: make system() block SIGCHLD

2012-01-15 Thread Richard Braun
On Sun, Jan 15, 2012 at 08:36:55PM +0100, Richard Braun wrote: I agree the use of signal() immediately disturbed me, but I didn't read its implementation. Maybe it does the job. In any case, we could use the occasion to fix that as well and replace signal() with sigaction() in the same patch.

Re: [PATCH] libc: make system() block SIGCHLD

2012-01-15 Thread Mike Frysinger
On Sunday 15 January 2012 14:36:55 Richard Braun wrote: On Sun, Jan 15, 2012 at 10:04:58AM -0500, Rich Felker wrote: Your report is wrong. system is REQUIRED by POSIX to change the signal disposition for SIGCHLD, not just to block the signal. It should change it to SIG_IGN, not SIG_DFL, but

Re: [PATCH] libc: make system() block SIGCHLD

2012-01-15 Thread Mike Frysinger
On Sunday 15 January 2012 14:50:24 Richard Braun wrote: On Sun, Jan 15, 2012 at 08:36:55PM +0100, Richard Braun wrote: I agree the use of signal() immediately disturbed me, but I didn't read its implementation. Maybe it does the job. In any case, we could use the occasion to fix that as

Re: [PATCH] libc: make system() block SIGCHLD

2012-01-14 Thread Mike Frysinger
On Friday 13 January 2012 22:09:04 Rich Felker wrote: In particular, in a multi-threaded program also from that POSIX page: * The system() function need not be thread-safe. makes me wonder if we shouldn't just gut the pthread locking from the NPTL version that was straight copied from

Re: [PATCH] libc: make system() block SIGCHLD

2012-01-14 Thread Mike Frysinger
On Wednesday 11 January 2012 12:37:31 Richard Braun wrote: When built with an old thread implementation (or for a sparc target), the implementation of the system() function doesn't conform to its specification. Namely, it resets the SIGCHLD handler to its default instead of blocking the

Re: [PATCH] libc: make system() block SIGCHLD

2012-01-14 Thread Richard Braun
On Sat, Jan 14, 2012 at 06:33:55AM -0500, Mike Frysinger wrote: if you unify the error paths with goto's, do you get smaller code size ? Probably. I tried to follow the style of other code pieces I've read, which is why I avoided using gotos in the first place. -- Richard Braun

[PATCH] libc: make system() block SIGCHLD

2012-01-14 Thread Richard Braun
When built with an old thread implementation (or for a sparc target), the implementation of the system() function doesn't conform to its specification. Namely, it resets the SIGCHLD handler to its default instead of blocking the signal, which may result in lost signals if a custom handler was

Re: [PATCH] libc: make system() block SIGCHLD

2012-01-14 Thread Richard Braun
On Sat, Jan 14, 2012 at 02:03:45PM +0100, Richard Braun wrote: + if (sigprocmask(SIG_SETMASK, omask, NULL) != 0) + ret_val = -1; If you consider there is no need to check the return value of sigprocmask() (which should only fail if parameters are invalid AIUI), feel free to

Re: [PATCH] libc: make system() block SIGCHLD

2012-01-14 Thread Mike Frysinger
On Saturday 14 January 2012 07:39:11 Richard Braun wrote: On Sat, Jan 14, 2012 at 06:33:55AM -0500, Mike Frysinger wrote: if you unify the error paths with goto's, do you get smaller code size ? Probably. I tried to follow the style of other code pieces I've read, which is why I avoided

Re: [PATCH] libc: make system() block SIGCHLD

2012-01-14 Thread Mike Frysinger
On Saturday 14 January 2012 08:21:27 Richard Braun wrote: On Sat, Jan 14, 2012 at 02:03:45PM +0100, Richard Braun wrote: + if (sigprocmask(SIG_SETMASK, omask, NULL) != 0) + ret_val = -1; If you consider there is no need to check the return value of sigprocmask() (which should

Re: [PATCH] libc: make system() block SIGCHLD

2012-01-13 Thread Rich Felker
On Wed, Jan 11, 2012 at 06:37:31PM +0100, Richard Braun wrote: When built with an old thread implementation (or for a sparc target), the implementation of the system() function doesn't conform to its specification. Namely, it resets the SIGCHLD handler to its default instead of blocking the

Re: [PATCH] libc: make system() block SIGCHLD

2012-01-13 Thread Richard Braun
On Fri, Jan 13, 2012 at 10:09:04PM -0500, Rich Felker wrote: sigprocmask alone is not sufficient to implement the required behavior for system. In particular, in a multi-threaded program, another thread will receive the signal and reap the child, causing system to malfunction. Really there's

[PATCH] libc: make system() block SIGCHLD

2012-01-11 Thread Richard Braun
When built with an old thread implementation (or for a sparc target), the implementation of the system() function doesn't conform to its specification. Namely, it resets the SIGCHLD handler to its default instead of blocking the signal, which may result in lost signals if a custom handler was