Re: [PATCH] v3 BZ# 18125: setcontext: Call exit, not _exit, after last linked context executes.

["Carlos O'Donell" <carlos at redhat dot com>]

On 03/23/2015 11:59 AM, Carlos O'Donell wrote:
> On 03/17/2015 03:58 AM, Mike Frysinger wrote:
>> On 16 Mar 2015 19:14, Carlos O'Donell wrote:
>>> 	* Makefile (tests): Add tst-setcontext3.
>>
>> your ChangeLog says Makefile, but the patch doesn't contain it ...
> 
> Fixed. Attached.
>  
>>> --- /dev/null
>>> +++ b/stdlib/tst-setcontext3.c
>>>
>>> +char *filename;
>>
>> could be static right ?
> 
> Fixed. Made static.
> 
>>> +static int
>>> +do_test (int argc, char **argv)
>>> +{
>>> +  int ret;
>>> +  char st1[32768];
>>> +  ucontext_t tempctx = ctx;
>>> +
>>> +  if (argc < 2)
>>> +    {
>>> +      printf ("FAIL: Test missing filename argument.\n");
>>> +      exit (1);
>>
>> it's not wrong, just weird, but this func uses exit half the time and return the 
>> other half ...
> 
> Fixed. All use exit().
> 
>>> --- /dev/null
>>> +++ b/stdlib/tst-setcontext3.sh
>>>
>>> +cleanup() {
>>> +    rm -f "${tempfiles[@]}"
>>> +}
>>
>> stylewise, seems like we normally use two space indent ?
> 
> Fixed. Use two space indent.
> 
>>> +# We want to run the test program and see if secontext called
>>> +# exit() and wrote out the test file we specified.  If the
>>> +# test exits with a non-zero status this will fail because we
>>> +# are using `set -e`.
>>> +$test_pre $test "$tempfile"
>>
>> what about 77 ?  the test_pre part really only ever expands into `env VAR=val` 
>> right ?  so i think you need to explicitly capture & test the exit value in 
>> order to handle 77 correctly.
> 
> The script uses `set -e` which means any failure in an executed subshell
> immediately causes the present shell to exit with that error. The rules that are
> running tests can detect an exit code of 77 and translate that into UNSUPPORTED.
> I tested this by hand by making the test exit (77) immediately and it is correctly
> reported as an UNSUPPORTED test.
> 
> If nobody objects to this version, this is what I'll commit shortly. It brings
> us a step closer to correctly exiting the process, but does not resolve the the
> fact that the standard says "thread" and thus means we should be calling pthread_exit.
> I don't have any opinion on that right now.
> 
> My goal here to mainly to align AArch64 with x86_64 in terms of functionality.
> 
> v3
> - Attach Makefile diff.
> - Correct Changelog.
> - Make filename static.
> - Use exit() everywhere.
> - Use 2 space indent in shell.
> - Update shell wrapper to call tst-setcontext3.
> 
> Cheers,
> Carlos.
> 
> 2015-03-23  Carlos O'Donell  <carlos@redhat.com>
> 
> 	[BZ #18125]
> 	* stdlib/tst-setcontext3.c: New file.
> 	* stdlib/tst-setcontext3.sh: New file.
> 	* stdlib/Makefile (tests): Add tst-setcontext3.
> 	(tst-setcontext3.out): Custom rule to run tst-setcontext3.sh
> 	to verify test program created output file.
> 	* sysdeps/unix/sysv/linux/aarch64/setcontext.S: Call exit.
> 	* sysdeps/unix/sysv/linux/arm/setcontext.S: Likewise.
> 	* sysdeps/unix/sysv/linux/hppa/setcontext.S: Likewise.
> 	* sysdeps/unix/sysv/linux/nios2/setcontext.S: Likewise.

Checked in.

Cheers,
Carlos.