[PATCH] C11 threads: Fix inaccuracies in testsuite

[PATCH] C11 threads: Fix inaccuracies in testsuite

[Corinna Vinschen]

- 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(-)

diff --git a/sysdeps/pthread/tst-mtx-recursive.c b/sysdeps/pthread/tst-mtx-recursive.c
index 6b471ac724df..aca8cee6eb99 100644
--- a/sysdeps/pthread/tst-mtx-recursive.c
+++ b/sysdeps/pthread/tst-mtx-recursive.c
@@ -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)
diff --git a/sysdeps/pthread/tst-thrd-sleep.c b/sysdeps/pthread/tst-thrd-sleep.c
index 39d5fc707945..8cc4bb2690c4 100644
--- a/sysdeps/pthread/tst-thrd-sleep.c
+++ b/sysdeps/pthread/tst-thrd-sleep.c
@@ -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);
diff --git a/sysdeps/pthread/tst-tss-basic.c b/sysdeps/pthread/tst-tss-basic.c
index 3b06abc5cfe5..5a2c1bd1ee3e 100644
--- a/sysdeps/pthread/tst-tss-basic.c
+++ b/sysdeps/pthread/tst-tss-basic.c
@@ -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);
-- 
2.26.2

Re: [PATCH] C11 threads: Fix inaccuracies in testsuite

[Florian Weimer]

* 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

Re: [PATCH] C11 threads: Fix inaccuracies in testsuite

[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.

> > 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
Cygwin Maintainer
Red Hat

Re: [PATCH] C11 threads: Fix inaccuracies in testsuite

[Florian Weimer]

* 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

Re: [PATCH] C11 threads: Fix inaccuracies in testsuite

[Corinna Vinschen]

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
Cygwin Maintainer
Red Hat

Re: [PATCH] C11 threads: Fix inaccuracies in testsuite

[Florian Weimer]

* Corinna Vinschen via Libc-alpha:

> I didn't make any special arrangements, so this should be ok.

Thanks, I've pushed your patch.

Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill