| Message ID | 20200830114436.1171468-1-vinschen@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | C11 threads: Fix inaccuracies in testsuite | expand |
* 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
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);
- 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(-)