Re: Fix nptl/tst-cancel7 for non-bash shells

["Carlos O'Donell" <carlos at systemhalted dot org>]

On Mon, Oct 29, 2012 at 4:38 PM, Rich Felker <dalias@aerifal.cx> wrote:
> On Mon, Oct 29, 2012 at 04:18:11PM -0400, Carlos O'Donell wrote:
>> On Mon, Oct 29, 2012 at 12:02 PM, Rich Felker <dalias@aerifal.cx> wrote:
>> > On Mon, Oct 29, 2012 at 09:37:34AM -0400, Carlos O'Donell wrote:
>> >> On Sun, Oct 28, 2012 at 9:15 PM, Rich Felker <dalias@aerifal.cx> wrote:
>> >> > This issue caused me to take a look at the implementation of system's
>> >> > handling of cancellation, which turns out to have a serious bug:
>> >> >
>> >> > http://sourceware.org/bugzilla/show_bug.cgi?id=14782
>> >> >
>> >> > It should be easy to fix; the fix is a purely-red patch (all -'s).
>> >>
>> >> While your analysis looks correct the more interesting question to me
>> >> is "Why was this added in the first place?" Can you dig a bit more and
>> >> see if we can determine the reason for the deviation from the standard?
>> >> Clearly we went to a lot of work to implement the current behaviour and
>> >> I'd like to know why.
>> >
>> > Where would you suggest I start? "system" is not the easiest function
>> > to search the bug tracker for, but I just did and the only resolved
>> > bugs I found related to system were invalid (requests for it to close
>> > the parent's file descriptors or catch the SIGCHLD). As such, I don't
>> > think this code was written to fix any bug.
>>
>> I recommend:
>>
>> (1) Use gitweb to browse the history of the changes in the files
>>     and lines of code that implement the change. Alternatively use
>>     `git blame`, `git log` and `git diff ...`.
>
> This turned up commit 6ee8d3345646ab0bea91891362a2bbf15503edec
>
> http://sourceware.org/git/?p=glibc.git;a=commit;f=sysdeps/posix/system.c;h=6ee8d3345646ab0bea91891362a2bbf15503edec
>
> The commit was by Drepper and he seems to have applied this exact same
> pattern to a number of files to add cancellation support. The rest
> were all syscalls; including system() seems to have just been a
> mistake.

Thanks for tracking that down. I agree that it looks like a mechanical
application of the same solution to a bunch of files.

Given that, please feel free to workup a patch to fix this in system
and repost without hijacking Joseph's thread any further.

Cheers,
Carlos.