C11 threads: Fix inaccuracies in testsuite
Commit Message
- tst-mtx-recursive.c: mtx_init fails to use mtx_plain. Per C11
specs, using mtx_recursive alone is not supported. This isn't
catched because mtx_plain is defined as 0.
- tst-thrd-sleep.c: thrd_sleep() returns 0 on success, a negative
value on failure. Testing against thrd_success is incorrect.
- tst-tss-basic.c: tss_set() is incorrectly checkd for a non-0
value. The test should test aginst C11 threads error codes.
This isn't catched because thrd_success is defined as 0.
Note that all three tests fail on FreeBSD, which defines all mutex type
values, as well as all C11 threads error codes with non-0 values.
Signed-off-by: Corinna Vinschen <vinschen@redhat.com>
---
sysdeps/pthread/tst-mtx-recursive.c | 2 +-
sysdeps/pthread/tst-thrd-sleep.c | 2 +-
sysdeps/pthread/tst-tss-basic.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
Comments
* Corinna Vinschen via Libc-alpha:
> - tst-mtx-recursive.c: mtx_init fails to use mtx_plain. Per C11
> specs, using mtx_recursive alone is not supported. This isn't
> catched because mtx_plain is defined as 0.
>
> - tst-thrd-sleep.c: thrd_sleep() returns 0 on success, a negative
> value on failure. Testing against thrd_success is incorrect.
>
> - tst-tss-basic.c: tss_set() is incorrectly checkd for a non-0
> value. The test should test aginst C11 threads error codes.
> This isn't catched because thrd_success is defined as 0.
>
> Note that all three tests fail on FreeBSD, which defines all mutex type
> values, as well as all C11 threads error codes with non-0 values.
The patch looks good to me. Is this correctly described in the glibc
manual? I have checked, and I think the manual is correct, but it would
be nice if you could double-check it.
> Signed-off-by: Corinna Vinschen <vinschen@redhat.com>
I assume this is covered by the Red Hat copyright assignment?
(We do not use DCO in glibc, sorry.)
Thanks,
Florian
On Aug 31 10:59, Florian Weimer wrote:
> * Corinna Vinschen via Libc-alpha:
>
> > - tst-mtx-recursive.c: mtx_init fails to use mtx_plain. Per C11
> > specs, using mtx_recursive alone is not supported. This isn't
> > catched because mtx_plain is defined as 0.
> >
> > - tst-thrd-sleep.c: thrd_sleep() returns 0 on success, a negative
> > value on failure. Testing against thrd_success is incorrect.
> >
> > - tst-tss-basic.c: tss_set() is incorrectly checkd for a non-0
> > value. The test should test aginst C11 threads error codes.
> > This isn't catched because thrd_success is defined as 0.
> >
> > Note that all three tests fail on FreeBSD, which defines all mutex type
> > values, as well as all C11 threads error codes with non-0 values.
>
> The patch looks good to me. Is this correctly described in the glibc
> manual? I have checked, and I think the manual is correct, but it would
> be nice if you could double-check it.
Checked. The manual describes the parameters and return values
correctly.
> > Signed-off-by: Corinna Vinschen <vinschen@redhat.com>
>
> I assume this is covered by the Red Hat copyright assignment?
I don't know, I assumed this.
> (We do not use DCO in glibc, sorry.)
I don't know what DCO is, sorry.
Thanks,
Corinna
* Corinna Vinschen:
> On Aug 31 10:59, Florian Weimer wrote:
>> * Corinna Vinschen via Libc-alpha:
>>
>> > - tst-mtx-recursive.c: mtx_init fails to use mtx_plain. Per C11
>> > specs, using mtx_recursive alone is not supported. This isn't
>> > catched because mtx_plain is defined as 0.
>> >
>> > - tst-thrd-sleep.c: thrd_sleep() returns 0 on success, a negative
>> > value on failure. Testing against thrd_success is incorrect.
>> >
>> > - tst-tss-basic.c: tss_set() is incorrectly checkd for a non-0
>> > value. The test should test aginst C11 threads error codes.
>> > This isn't catched because thrd_success is defined as 0.
>> >
>> > Note that all three tests fail on FreeBSD, which defines all mutex type
>> > values, as well as all C11 threads error codes with non-0 values.
>>
>> The patch looks good to me. Is this correctly described in the glibc
>> manual? I have checked, and I think the manual is correct, but it would
>> be nice if you could double-check it.
>
> Checked. The manual describes the parameters and return values
> correctly.
Thanks.
>> > Signed-off-by: Corinna Vinschen <vinschen@redhat.com>
>>
>> I assume this is covered by the Red Hat copyright assignment?
>
> I don't know, I assumed this.
It will be, unless you have made special arrangements with Red Hat about
who owns copyright in the works you create for Red Hat.
>> (We do not use DCO in glibc, sorry.)
>
> I don't know what DCO is, sorry.
Adding the Signed-off-by line is expected to indicate that you agree to
the DCO terms:
-s, --signoff
Add Signed-off-by line by the committer at the end of the
commit log message. The meaning of a signoff depends on the
project, but it typically certifies that committer has the
rights to submit this work under the same license and agrees
to a Developer Certificate of Origin (see
http://developercertificate.org/ for more information).
So I suspect this is not working as intended at all. 8-/
I'm going to apply the patch for you, without the Signed-off-by line.
Thanks,
Florian
On Aug 31 12:16, Florian Weimer wrote:
> * Corinna Vinschen:
>
> > On Aug 31 10:59, Florian Weimer wrote:
> >> * Corinna Vinschen via Libc-alpha:
> >>
> >> > - tst-mtx-recursive.c: mtx_init fails to use mtx_plain. Per C11
> >> > specs, using mtx_recursive alone is not supported. This isn't
> >> > catched because mtx_plain is defined as 0.
> >> >
> >> > - tst-thrd-sleep.c: thrd_sleep() returns 0 on success, a negative
> >> > value on failure. Testing against thrd_success is incorrect.
> >> >
> >> > - tst-tss-basic.c: tss_set() is incorrectly checkd for a non-0
> >> > value. The test should test aginst C11 threads error codes.
> >> > This isn't catched because thrd_success is defined as 0.
> >> >
> >> > Note that all three tests fail on FreeBSD, which defines all mutex type
> >> > values, as well as all C11 threads error codes with non-0 values.
> >>
> >> The patch looks good to me. Is this correctly described in the glibc
> >> manual? I have checked, and I think the manual is correct, but it would
> >> be nice if you could double-check it.
> >
> > Checked. The manual describes the parameters and return values
> > correctly.
>
> Thanks.
>
> >> > Signed-off-by: Corinna Vinschen <vinschen@redhat.com>
> >>
> >> I assume this is covered by the Red Hat copyright assignment?
> >
> > I don't know, I assumed this.
>
> It will be, unless you have made special arrangements with Red Hat about
> who owns copyright in the works you create for Red Hat.
I didn't make any special arrangements, so this should be ok.
Corinna
* Corinna Vinschen via Libc-alpha:
> I didn't make any special arrangements, so this should be ok.
Thanks, I've pushed your patch.
Florian
@@ -27,7 +27,7 @@ do_test (void)
{
static mtx_t mutex;
- if (mtx_init (&mutex, mtx_recursive) != thrd_success)
+ if (mtx_init (&mutex, mtx_plain | mtx_recursive) != thrd_success)
FAIL_EXIT1 ("mtx_init failed");
if (mtx_lock (&mutex) != thrd_success)
@@ -27,7 +27,7 @@ static int
sleep_thrd (void *arg)
{
struct timespec const *tl = (struct timespec const *) arg;
- if (thrd_sleep (tl, NULL) != thrd_success)
+ if (thrd_sleep (tl, NULL) != 0)
FAIL_EXIT1 ("thrd_sleep failed");
thrd_exit (thrd_success);
@@ -33,7 +33,7 @@ tss_thrd (void *arg)
if (tss_create (&key, NULL) != thrd_success)
FAIL_EXIT1 ("tss_create failed");
- if (tss_set (key, TSS_VALUE))
+ if (tss_set (key, TSS_VALUE) != thrd_success)
FAIL_EXIT1 ("tss_set failed");
void *value = tss_get (key);