Subject: Re: lib/31320: pthread_cleanup_push and pthread_cleanup_pop macros broken
To: None <gnats-bugs@NetBSD.org>
From: Nathan J. Williams <nathanw@wasabisystems.com>
List: netbsd-bugs
Date: 09/15/2005 10:55:23
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