This is the mail archive of the libc-hacker@sourceware.org mailing list for the glibc project.
Note that libc-hacker is a closed list. You may look at the archives of this list, but subscription and posting are not open.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
Hi! As can be seen on the attached testcase, pthread_condattr_setclock.c is wrong, in addition to setting the desired clock it also sets pshared to PTHREAD_PROCESS_PRIVATE. COND_NWAITERS_SHIFT is ATM 1, so (*valuep & 1) is pshared bit and (*valuep & 2) >> 1 is the clock id bit. So, to just set the clock id and not anything else, pthread_condattr_setclock wants to do *valuep = (*valuep & ~2) | (clock_id << 1), but it actually does: *valuep = (*valuep & ~4 & ~1) | (clock_id << 1) i.e. *valuep = (*valuep & ~5) | (clock_id << 1). Also, in pthread_condattr_setclock (&ca, CLOCK_MONOTONIC); pthread_condattr_setclock (&ca, CLOCK_REALTIME); the second pthread_condattr_setclock will not do anything (except for acting as pthread_condattr_setpshared (&ca, PTHREAD_PROCESS_PRIVATE);), clock id will remain CLOCK_MONOTONIC. The following patch uses *valuep = (*valuep & ~(((1 << COND_NWAITERS_SHIFT) - 1) << 1)) | (clock_id << 1) i.e. *valuep = (*valuep & ~(((1 << 1) - 1) << 1)) | (clock_id << 1) *valuep = (*valuep & ~2) | (clock_id << 1) Alternatively, we could use: *valuep = (*valuep & ~(((1 << (COND_NWAITERS_SHIFT + 1)) - 1) & ~1)) | (clock_id << 1) which is also *valuep = (*valuep & ~2) | (clock_id << 1) in the end, but that's IMHO longer and less readable. This patch also fixes pthread_cond_init, which has very weird expression, which happens to be correct for COND_NWAITERS_SHIFT == 1, but is wrong for all the other values of that macro. 2008-11-12 Jakub Jelinek <jakub@redhat.com> [BZ #7008] * pthread_condattr_setclock.c (pthread_condattr_setclock): Fix masking of old value. * pthread_cond_init.c (__pthread_cond_init): Fix cond->__data.__nwaiters initialization. * Makefile (tests): Add tst-cond23. * tst-cond23.c: New test. --- libc/nptl/pthread_condattr_setclock.c.jj 2007-06-04 08:42:05.000000000 +0200 +++ libc/nptl/pthread_condattr_setclock.c 2008-11-12 09:36:03.000000000 +0100 @@ -1,4 +1,4 @@ -/* Copyright (C) 2003, 2004, 2007 Free Software Foundation, Inc. +/* Copyright (C) 2003, 2004, 2007, 2008 Free Software Foundation, Inc. This file is part of the GNU C Library. Contributed by Ulrich Drepper <drepper@redhat.com>, 2003. @@ -66,7 +66,7 @@ pthread_condattr_setclock (attr, clock_i int *valuep = &((struct pthread_condattr *) attr)->value; - *valuep = ((*valuep & ~(1 << (COND_NWAITERS_SHIFT + 1)) & ~1) + *valuep = ((*valuep & ~(((1 << COND_NWAITERS_SHIFT) - 1) << 1)) | (clock_id << 1)); return 0; --- libc/nptl/pthread_cond_init.c.jj 2007-08-01 10:50:19.000000000 +0200 +++ libc/nptl/pthread_cond_init.c 2008-11-12 09:15:34.000000000 +0100 @@ -1,4 +1,5 @@ -/* Copyright (C) 2002, 2003, 2004, 2005, 2007 Free Software Foundation, Inc. +/* Copyright (C) 2002, 2003, 2004, 2005, 2007, 2008 + Free Software Foundation, Inc. This file is part of the GNU C Library. Contributed by Ulrich Drepper <drepper@redhat.com>, 2002. @@ -31,8 +32,9 @@ __pthread_cond_init (cond, cond_attr) cond->__data.__lock = LLL_LOCK_INITIALIZER; cond->__data.__futex = 0; cond->__data.__nwaiters = (icond_attr != NULL - && ((icond_attr->value - & (COND_NWAITERS_SHIFT << 1)) >> 1)); + ? ((icond_attr->value >> 1) + & ((1 << COND_NWAITERS_SHIFT) - 1)) + : CLOCK_REALTIME); cond->__data.__total_seq = 0; cond->__data.__wakeup_seq = 0; cond->__data.__woken_seq = 0; --- libc/nptl/Makefile.jj 2008-07-08 12:57:27.000000000 +0200 +++ libc/nptl/Makefile 2008-11-12 10:19:46.000000000 +0100 @@ -205,7 +205,7 @@ tests = tst-typesizes \ tst-cond1 tst-cond2 tst-cond3 tst-cond4 tst-cond5 tst-cond6 tst-cond7 \ tst-cond8 tst-cond9 tst-cond10 tst-cond11 tst-cond12 tst-cond13 \ tst-cond14 tst-cond15 tst-cond16 tst-cond17 tst-cond18 tst-cond19 \ - tst-cond20 tst-cond21 tst-cond22 \ + tst-cond20 tst-cond21 tst-cond22 tst-cond23 \ tst-robust1 tst-robust2 tst-robust3 tst-robust4 tst-robust5 \ tst-robust6 tst-robust7 tst-robust8 tst-robust9 \ tst-robustpi1 tst-robustpi2 tst-robustpi3 tst-robustpi4 tst-robustpi5 \ --- libc/nptl/tst-cond23.c.jj 2008-11-12 09:52:28.000000000 +0100 +++ libc/nptl/tst-cond23.c 2008-11-12 10:19:07.000000000 +0100 @@ -0,0 +1,184 @@ +/* Copyright (C) 2008 Free Software Foundation, Inc. + This file is part of the GNU C Library. + Contributed by Jakub Jelinek <jakub@redhat.com>, 2008. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, write to the Free + Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA + 02111-1307 USA. */ + +#include <errno.h> +#include <pthread.h> +#include <stdio.h> +#include <time.h> +#include <unistd.h> + + +#if defined _POSIX_CLOCK_SELECTION && _POSIX_CLOCK_SELECTION >= 0 +static int +check (pthread_condattr_t *condattr, int pshared, clockid_t cl) +{ + clockid_t cl2; + if (pthread_condattr_getclock (condattr, &cl2) != 0) + { + puts ("condattr_getclock failed"); + return 1; + } + if (cl != cl2) + { + printf ("condattr_getclock returned wrong value: %d, expected %d\n", + (int) cl2, (int) cl); + return 1; + } + + int p; + if (pthread_condattr_getpshared (condattr, &p) != 0) + { + puts ("condattr_getpshared failed"); + return 1; + } + else if (p != pshared) + { + printf ("condattr_getpshared returned wrong value: %d, expected %d\n", + p, pshared); + return 1; + } + + return 0; +} + +static int +run_test (clockid_t cl) +{ + pthread_condattr_t condattr; + + printf ("clock = %d\n", (int) cl); + + if (pthread_condattr_init (&condattr) != 0) + { + puts ("condattr_init failed"); + return 1; + } + + if (check (&condattr, PTHREAD_PROCESS_PRIVATE, CLOCK_REALTIME)) + return 1; + + if (pthread_condattr_setpshared (&condattr, PTHREAD_PROCESS_SHARED) != 0) + { + puts ("1st condattr_setpshared failed"); + return 1; + } + + if (check (&condattr, PTHREAD_PROCESS_SHARED, CLOCK_REALTIME)) + return 1; + + if (pthread_condattr_setclock (&condattr, cl) != 0) + { + puts ("1st condattr_setclock failed"); + return 1; + } + + if (check (&condattr, PTHREAD_PROCESS_SHARED, cl)) + return 1; + + if (pthread_condattr_setpshared (&condattr, PTHREAD_PROCESS_PRIVATE) != 0) + { + puts ("2nd condattr_setpshared failed"); + return 1; + } + + if (check (&condattr, PTHREAD_PROCESS_PRIVATE, cl)) + return 1; + + if (pthread_condattr_setclock (&condattr, CLOCK_REALTIME) != 0) + { + puts ("2nd condattr_setclock failed"); + return 1; + } + + if (check (&condattr, PTHREAD_PROCESS_PRIVATE, CLOCK_REALTIME)) + return 1; + + if (pthread_condattr_setclock (&condattr, cl) != 0) + { + puts ("3rd condattr_setclock failed"); + return 1; + } + + if (check (&condattr, PTHREAD_PROCESS_PRIVATE, cl)) + return 1; + + if (pthread_condattr_setpshared (&condattr, PTHREAD_PROCESS_SHARED) != 0) + { + puts ("3rd condattr_setpshared failed"); + return 1; + } + + if (check (&condattr, PTHREAD_PROCESS_SHARED, cl)) + return 1; + + if (pthread_condattr_setclock (&condattr, CLOCK_REALTIME) != 0) + { + puts ("4th condattr_setclock failed"); + return 1; + } + + if (check (&condattr, PTHREAD_PROCESS_SHARED, CLOCK_REALTIME)) + return 1; + + if (pthread_condattr_destroy (&condattr) != 0) + { + puts ("condattr_destroy failed"); + return 1; + } + + return 0; +} +#endif + + +static int +do_test (void) +{ +#if !defined _POSIX_CLOCK_SELECTION || _POSIX_CLOCK_SELECTION == -1 + + puts ("_POSIX_CLOCK_SELECTION not supported, test skipped"); + return 0; + +#else + + int res = run_test (CLOCK_REALTIME); + +# if defined _POSIX_MONOTONIC_CLOCK && _POSIX_MONOTONIC_CLOCK >= 0 +# if _POSIX_MONOTONIC_CLOCK == 0 + int e = sysconf (_SC_MONOTONIC_CLOCK); + if (e < 0) + puts ("CLOCK_MONOTONIC not supported"); + else if (e == 0) + { + puts ("sysconf (_SC_MONOTONIC_CLOCK) must not return 0"); + res = 1; + } + else +# endif + res |= run_test (CLOCK_MONOTONIC); +# else + puts ("_POSIX_MONOTONIC_CLOCK not defined"); +# endif + + return res; +#endif +} + +#define TEST_FUNCTION do_test () +#include "../test-skeleton.c" Jakub
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |