Re: [PATCH 2/4] nptl: Using libsupport for tst-cancel4

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

On 01/05/2017 12:31, Wainer dos Santos Moschetta wrote:
>> -      strerror (errno));
>> -
>> -  exit (1);
>> +  FAIL_EXIT1 ("select returns with %d: %m", s);
>>   }
>>     @@ -418,23 +353,17 @@ tf_pselect (void *arg)
>>         char fname[] = "/tmp/tst-cancel4-fd-XXXXXX";
>>         tempfd = fd = mkstemp (fname);
>>         if (fd == -1)
>> -    printf ("%s: mkstemp failed\n", __FUNCTION__);
>> +    FAIL_EXIT1 ("mkstemp");
> 
> Perhaps use...
> 
> FAIL_EXIT1 ("mkstemp failed: %m");
> 
> ... like in many occurrences above. Or perhaps use:
> 
> FAIL_EXIT1 ("mkstemp: %m");
> 
> Unless the error string isn't important.

Ack, I will use the first definition for consistency.


>> @@ -943,9 +757,7 @@ tf_pause (void *arg)
>>       pthread_cleanup_pop (0);
>>   -  printf ("%s: pause returned\n", __FUNCTION__);
>> -
>> -  exit (1);
>> +  FAIL_EXIT1 ("pause returned");
>>   }
>>     @@ -959,25 +771,18 @@ tf_accept (void *arg)
>>       tempfd = socket (AF_UNIX, pf, 0);
>>     if (tempfd == -1)
>> -    {
>> -      printf ("%s: socket call failed\n", __FUNCTION__);
>> -      exit (1);
>> -    }
>> +    FAIL_EXIT1 ("socket (AF_UNIX, %s, 0): %m", arg == NULL ? "SOCK_STREAM"
>> +                           : "SOCK_DGRAM");
>>       int tries = 0;
>>     do
>>       {
>>         if (++tries > 10)
>> -    {
>> -      printf ("%s: too many unsuccessful bind calls\n", __FUNCTION__);
>> -    }
>> +    FAIL_EXIT1 ("too many unsuccessful bind calls");
> 
> really want to exit here?

We need some fallback mechanism for the very unlikely case of having all the
files with the template name used on mkstemp already used to avoid an infinite
loop.  Usually 10 times should be suffice, since it should success on first
try.

>>   -  printf ("%s: fdatasync returned\n", __FUNCTION__);
>> -
>> -  exit (1);
>> +  FAIL_EXIT1 ("fdatasync returned");
>>   }
>>     @@ -1693,30 +1326,19 @@ tf_msync (void *arg)
>>       tempfd = open ("Makefile", O_RDONLY);
>>     if (tempfd == -1)
>> -    {
>> -      printf ("%s: cannot open Makefile\n", __FUNCTION__);
>> -      exit (1);
>> -    }
>> +    FAIL_EXIT1 ("open (\"Makefile\", O_RDONLY): %m");
>> +
> 
> Wondering if wouldn't better replace above block with TEST_VERIFY_EXIT.  Many occurrences throughout this patch have such as pattern though.

It really depends of what kind of error failure information we want to
inform.  FAIL_EXIT1 allows a variadic argument to allow more informative
strings, where TEST_VERIFY_EXIT just prints the 'stringification' of the
test passed as argument.  I think for mmap we may use, although for
open I prefer to still printing the resulting errno information (thorough
%m).

> 
>>     void *p = mmap (NULL, 10, PROT_READ, MAP_SHARED, tempfd, 0);
>>     if (p == MAP_FAILED)
>> -    {
>> -      printf ("%s: mmap failed\n", __FUNCTION__);
>> -      exit (1);
>> -    }
>> +    FAIL_EXIT1 ("mmap (NULL, 10, PROT_READ, MAP_SHARED, ...): %m");
> 
> Likewise, etc..