Subject: Re: lib/31320: pthread_cleanup_push and pthread_cleanup_pop macros broken
To: None <lib-bug-people@netbsd.org, gnats-admin@netbsd.org,>
From: Nathan J. Williams <nathanw@wasabisystems.com>
List: netbsd-bugs
Date: 09/15/2005 15:11:01
The following reply was made to PR lib/31320; it has been noted by GNATS.

From: "Nathan J. Williams" <nathanw@wasabisystems.com>
To: gnats-bugs@NetBSD.org
Cc: lib-bug-people@NetBSD.org, gnats-admin@NetBSD.org,
	netbsd-bugs@NetBSD.org
Subject: Re: lib/31320: pthread_cleanup_push and pthread_cleanup_pop macros broken
Date: 15 Sep 2005 10:55:23 -0400

 netbsd@wolfnode.de writes:
 
 > pthread_cleanup_push and pthread_cleanup_pop are macros from pthread.h.
 > 
 > They are currentely implemented as:
 > 
 > #define pthread_cleanup_push(routine, arg)			\
 >         {							\
 > 		struct pthread_cleanup_store __store;		\
 > 		pthread__cleanup_push((routine),(arg), &__store);
 > 
 > #define pthread_cleanup_pop(execute)				\
 > 		pthread__cleanup_pop((execute), &__store);	\
 > 	}
 > 
 > 
 > I think the closing bracket from pop must move to push...
 
 No. They aren't intended to work as ordinary functions; you must use
 them in pairs. See
 http://www.opengroup.org/onlinepubs/009695399/functions/pthread_cleanup_pop.html
 
     These functions may be implemented as macros. The application
     shall ensure that they appear as statements, and in pairs within
     the same lexical scope (that is, the pthread_cleanup_push() macro
     may be thought to expand to a token list whose first token is '{'
     with pthread_cleanup_pop() expanding to a token list whose last
     token is the corresponding '}' ).
 
 
 
 > >How-To-Repeat:
 > void test(void* P)
 > {
 >     // Shutdown handler
 > }
 > 
 > void* threadMain(void* P)
 > {
 >     // This will *NOT* work...
 >     pthread_cleanup_push(test, P);
 > 
 >     // But this DOES work - which is wrong
 >     pthread_cleanup_push(test, P) }  // Yes, }
 > }
 
 What will work is:
 
 void* threadMain(void* P)
 {
     pthread_cleanup_push(test, P);
 
     pthread_cleanup_pop(0);
 }
 
 
 > >Fix:
 > Replace the macros by
 > 
 > #define pthread_cleanup_push(routine, arg)			\
 >         {							\
 > 		struct pthread_cleanup_store __store;		\
 > 		pthread__cleanup_push((routine),(arg), &__store); \
 >         }
 > 
 > #define pthread_cleanup_pop(execute)				\
 > 		pthread__cleanup_pop((execute), &__store);
 
 This will not compile, as the closing brace of the push routine will
 end the scope of __store.
 
         - Nathan