[PATCH 02/13] nptl: Fix testcases for new pthread cancellation mechanism
Adhemerval Zanella
adhemerval.zanella@linaro.org
Thu Oct 8 20:06:00 GMT 2015
Thanks for the review.
On 07-10-2015 17:59, Roland McGrath wrote:
>> diff --git a/nptl/tst-cancel2.c b/nptl/tst-cancel2.c
>> index 1a74992..3d34169 100644
>> --- a/nptl/tst-cancel2.c
>> +++ b/nptl/tst-cancel2.c
>> @@ -33,6 +33,9 @@ tf (void *arg)
>> char buf[100000];
>>
>> while (write (fd[1], buf, sizeof (buf)) > 0);
>> + /* The write can return a value higher than 0 (meaning partial write)
>> + due the SIGCANCEL, but the thread may still pending cancellation. */
>> + pthread_testcancel ();
>
> "due to the"
> "may still be pending"
>
> The same comment with the same errors is repeated in several tests.
I will fix that.
>
>> --- a/nptl/tst-cancel4.c
>> +++ b/nptl/tst-cancel4.c
>> @@ -38,8 +38,10 @@
>> #include <sys/un.h>
>> #include <sys/wait.h>
>>
>> -#include "pthreadP.h"
>> -
>> +/* The signal used for asynchronous cancelation. */
>> +#ifndef SIGCANCEL
>> +# define SIGCANCEL __SIGRTMIN
>> +#endif
>
> What's the rationale for this change? If the test needs SIGCANCEL then it
> is relying on implementation internals. If that is necessary for this
> test, then it is better to use an internal header than to duplicate part of
> its content. If some other part of pthreadP.h interferes with this code,
> or if you just want to avoid unnecessary internals, then you can just
> #include <nptl-signals.h> instead.
The rationale is to not tied the test with implementation internals. However
the sigpause test itself does not really require to block SIGCANCEL, we can
rather use another signal. I will change to 'sigpause (sigmask (SIGINT))'
and remote both the inclusion of 'pthreadP.h' and the SIGCANCEL definitions.
>
>> + if (arg == NULL)
>> + // XXX If somebody can provide a portable test case in which open()
>> + // blocks we can enable this test to run in both rounds.
>> + abort ();
>
> You can create such a scenario in POSIX systems using mkfifo.
>
Indeed (I just copies the block from previous tests), I will change that.
More information about the Libc-alpha
mailing list