| Message ID | 87o8upvweb.fsf@oldenburg2.str.redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | nptl: Avoid using PTHREAD_MUTEX_DEFAULT in macro definition [BZ #25271] | expand |
On 27/01/2020 10:53, Florian Weimer wrote: > Commit 1c3f9acf1f1f75faa7a28bf39af64afd ("nptl: Add struct_mutex.h") > replaced a zero constant with the identifier PTHREAD_MUTEX_DEFAULT > in the macro PTHREAD_MUTEX_INITIALIZER. However, that constant > is not available in ISO C11 mode: > > In file included from /usr/include/bits/thread-shared-types.h:74, > from /usr/include/bits/pthreadtypes.h:23, > from /usr/include/pthread.h:26, > from bug25271.c:1: > bug25271.c:3:21: error: ‘PTHREAD_MUTEX_DEFAULT’ undeclared here (not in a function) > 3 | pthread_mutex_t m = PTHREAD_MUTEX_INITIALIZER; > | ^~~~~~~~~~~~~~~~~~~~~~~~~ > > This commit change the constant to the equivalent > PTHREAD_MUTEX_TIMED_NP, which is in the POSIX extension namespace > and thus always available. > > Tested on x86_64-linux-gnu and i686-linux-gnu. Verified that a > minimal example now builds in -std=c11 mode. LGTM, although I think we should add some more extensive tests when 2.32 opens. > > ----- > sysdeps/nptl/pthread.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/sysdeps/nptl/pthread.h b/sysdeps/nptl/pthread.h > index 7825737840..44dd707896 100644 > --- a/sysdeps/nptl/pthread.h > +++ b/sysdeps/nptl/pthread.h > @@ -84,7 +84,7 @@ enum > > > #define PTHREAD_MUTEX_INITIALIZER \ > - { { __PTHREAD_MUTEX_INITIALIZER (PTHREAD_MUTEX_DEFAULT) } } > + { { __PTHREAD_MUTEX_INITIALIZER (PTHREAD_MUTEX_TIMED_NP) } } Maybe add a comment stating why PTHREAD_MUTEX_TIMED_NP is the correct initializer here? > #ifdef __USE_GNU > # define PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP \ > { { __PTHREAD_MUTEX_INITIALIZER (PTHREAD_MUTEX_RECURSIVE_NP) } } >
* Adhemerval Zanella: > On 27/01/2020 10:53, Florian Weimer wrote: >> Commit 1c3f9acf1f1f75faa7a28bf39af64afd ("nptl: Add struct_mutex.h") >> replaced a zero constant with the identifier PTHREAD_MUTEX_DEFAULT >> in the macro PTHREAD_MUTEX_INITIALIZER. However, that constant >> is not available in ISO C11 mode: >> >> In file included from /usr/include/bits/thread-shared-types.h:74, >> from /usr/include/bits/pthreadtypes.h:23, >> from /usr/include/pthread.h:26, >> from bug25271.c:1: >> bug25271.c:3:21: error: ‘PTHREAD_MUTEX_DEFAULT’ undeclared here (not in a function) >> 3 | pthread_mutex_t m = PTHREAD_MUTEX_INITIALIZER; >> | ^~~~~~~~~~~~~~~~~~~~~~~~~ >> >> This commit change the constant to the equivalent >> PTHREAD_MUTEX_TIMED_NP, which is in the POSIX extension namespace >> and thus always available. >> >> Tested on x86_64-linux-gnu and i686-linux-gnu. Verified that a >> minimal example now builds in -std=c11 mode. > > LGTM, although I think we should add some more extensive tests when > 2.32 opens. Thanks. Siddhesh, should we still put this into the release, or backport it afterwards? >> ----- >> sysdeps/nptl/pthread.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/sysdeps/nptl/pthread.h b/sysdeps/nptl/pthread.h >> index 7825737840..44dd707896 100644 >> --- a/sysdeps/nptl/pthread.h >> +++ b/sysdeps/nptl/pthread.h >> @@ -84,7 +84,7 @@ enum >> >> >> #define PTHREAD_MUTEX_INITIALIZER \ >> - { { __PTHREAD_MUTEX_INITIALIZER (PTHREAD_MUTEX_DEFAULT) } } >> + { { __PTHREAD_MUTEX_INITIALIZER (PTHREAD_MUTEX_TIMED_NP) } } > > Maybe add a comment stating why PTHREAD_MUTEX_TIMED_NP is the correct > initializer here? I don't want to add internal documentation to installed header files, in case an application programmer reads them. Thanks, Florian
On 30/01/20 2:53 pm, Florian Weimer wrote: >> LGTM, although I think we should add some more extensive tests when >> 2.32 opens. > > Thanks. Siddhesh, should we still put this into the release, or > backport it afterwards? > The fix is fine for master as long as you put it in today. I intend to start release testing Friday evening my time and cut the release branch on Saturday night. I'm about to announce a hard freeze for Friday evening (IST). Siddhesh
* Siddhesh Poyarekar: > On 30/01/20 2:53 pm, Florian Weimer wrote: >>> LGTM, although I think we should add some more extensive tests when >>> 2.32 opens. >> >> Thanks. Siddhesh, should we still put this into the release, or >> backport it afterwards? >> > > The fix is fine for master as long as you put it in today. I intend to > start release testing Friday evening my time and cut the release branch > on Saturday night. I'm about to announce a hard freeze for Friday > evening (IST). Thanks, I've pushed the original patch. Florian
diff --git a/sysdeps/nptl/pthread.h b/sysdeps/nptl/pthread.h index 7825737840..44dd707896 100644 --- a/sysdeps/nptl/pthread.h +++ b/sysdeps/nptl/pthread.h @@ -84,7 +84,7 @@ enum #define PTHREAD_MUTEX_INITIALIZER \ - { { __PTHREAD_MUTEX_INITIALIZER (PTHREAD_MUTEX_DEFAULT) } } + { { __PTHREAD_MUTEX_INITIALIZER (PTHREAD_MUTEX_TIMED_NP) } } #ifdef __USE_GNU # define PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP \ { { __PTHREAD_MUTEX_INITIALIZER (PTHREAD_MUTEX_RECURSIVE_NP) } }
Commit 1c3f9acf1f1f75faa7a28bf39af64afd ("nptl: Add struct_mutex.h") replaced a zero constant with the identifier PTHREAD_MUTEX_DEFAULT in the macro PTHREAD_MUTEX_INITIALIZER. However, that constant is not available in ISO C11 mode: In file included from /usr/include/bits/thread-shared-types.h:74, from /usr/include/bits/pthreadtypes.h:23, from /usr/include/pthread.h:26, from bug25271.c:1: bug25271.c:3:21: error: ‘PTHREAD_MUTEX_DEFAULT’ undeclared here (not in a function) 3 | pthread_mutex_t m = PTHREAD_MUTEX_INITIALIZER; | ^~~~~~~~~~~~~~~~~~~~~~~~~ This commit change the constant to the equivalent PTHREAD_MUTEX_TIMED_NP, which is in the POSIX extension namespace and thus always available. Tested on x86_64-linux-gnu and i686-linux-gnu. Verified that a minimal example now builds in -std=c11 mode. ----- sysdeps/nptl/pthread.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)