Date: Wed, 23 Oct 2013 12:12:37 +0200 From: Tijl Coosemans <tijl@FreeBSD.org> To: Jilles Tjoelker <jilles@stack.nl> Cc: multimedia-list freebsd <freebsd-multimedia@freebsd.org>, William Grzybowski <william88@gmail.com>, freebsd-standards@FreeBSD.org Subject: Re: VLC 2.1.0 Message-ID: <20131023121237.51e5a79e@kalimero.tijl.coosemans.org> In-Reply-To: <20131022212327.GB20055@stack.nl> References: <CAHtVNLONK3hZTUoJoebjtdZbhwK8pA8deO1um191zF5pZowF9A@mail.gmail.com> <20131022152502.61214646@kalimero.tijl.coosemans.org> <CAHtVNLOp%2BXqj%2BEbVsxDsLj4tjsxJkjWLm9G0ze_ND_noYGu_=Q@mail.gmail.com> <20131022174715.59433270@kalimero.tijl.coosemans.org> <20131022212327.GB20055@stack.nl>
next in thread | previous in thread | raw e-mail | index | archive | help
--MP_/pNkYODNbZBn4WVupizbFP3L
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
On Tue, 22 Oct 2013 23:23:27 +0200 Jilles Tjoelker wrote:
> On Tue, Oct 22, 2013 at 05:47:15PM +0200, Tijl Coosemans wrote:
>> Summarised: the idiom that VLC uses is this:
>>
>> pthread_cleanup_push(...);
>> ...
>> if (error) goto cleanup;
>> ...
>> cleanup:
>> pthread_cleanup_pop(...);
>>
>> Because the definition of the pthread_cleanup_pop macro starts with }
>> clang complains.
>
> glibc has do { } while (0); at the start of the pthread_cleanup_pop
> define. I think this is a better option than ; or (void)0; as it
> minimizes the wrong things it can combine with.
>
> Reading POSIX, it seems valid to put a label right before an invocation
> of pthread_cleanup_pop. In such a case, the invocation still appears as
> a statement and can be paired with a pthread_cleanup_push in the same
> lexical scope.
>
> Therefore the following patch to src seems appropriate.
>
> Index: include/pthread.h
> ===================================================================
> --- include/pthread.h (revision 256728)
> +++ include/pthread.h (working copy)
> @@ -175,6 +175,7 @@
> {
>
> #define pthread_cleanup_pop(execute) \
> + do { } while (0); \
> } \
> __pthread_cleanup_pop_imp(execute); \
> }
I had to think a bit about what could combine with (void)0; but not
do-while, but ?: can, so I agree. I've extended your patch further
to force the use of ;. Please review.
--MP_/pNkYODNbZBn4WVupizbFP3L
Content-Type: text/x-patch
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment; filename=pthread_cleanup.patch
Index: include/pthread.h
===================================================================
--- include/pthread.h (revision 256953)
+++ include/pthread.h (working copy)
@@ -168,17 +168,18 @@ int pthread_barrierattr_init(pthread_ba
int pthread_barrierattr_setpshared(pthread_barrierattr_t *, int);
#define pthread_cleanup_push(cleanup_routine, cleanup_arg) \
- { \
+ do { \
struct _pthread_cleanup_info __cleanup_info__; \
__pthread_cleanup_push_imp(cleanup_routine, cleanup_arg,\
&__cleanup_info__); \
- {
+ { \
+ do { } while(0)
#define pthread_cleanup_pop(execute) \
- (void)0; \
+ do { } while(0); \
} \
__pthread_cleanup_pop_imp(execute); \
- }
+ } while(0)
int pthread_condattr_destroy(pthread_condattr_t *);
int pthread_condattr_getclock(const pthread_condattr_t *,
--MP_/pNkYODNbZBn4WVupizbFP3L--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20131023121237.51e5a79e>