From owner-freebsd-standards@FreeBSD.ORG Wed Oct 23 10:12:41 2013 Return-Path: Delivered-To: freebsd-standards@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTP id D8E9B275; Wed, 23 Oct 2013 10:12:41 +0000 (UTC) (envelope-from tijl@freebsd.org) Received: from mailrelay011.isp.belgacom.be (mailrelay011.isp.belgacom.be [195.238.6.178]) by mx1.freebsd.org (Postfix) with ESMTP id 4E25822B6; Wed, 23 Oct 2013 10:12:41 +0000 (UTC) X-Belgacom-Dynamic: yes X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: AlQGAA2gZ1Jbs4WZ/2dsb2JhbABZgwe8XoJ6gSYXdIIlAQEFViMQCxgJJQ8qHgaIHQG6YY9OB4QqA5Ath1uSCIMmOg Received: from 153.133-179-91.adsl-dyn.isp.belgacom.be (HELO kalimero.tijl.coosemans.org) ([91.179.133.153]) by relay.skynet.be with ESMTP; 23 Oct 2013 12:12:39 +0200 Received: from kalimero.tijl.coosemans.org (kalimero.tijl.coosemans.org [127.0.0.1]) by kalimero.tijl.coosemans.org (8.14.7/8.14.7) with ESMTP id r9NACcqk009372; Wed, 23 Oct 2013 12:12:38 +0200 (CEST) (envelope-from tijl@FreeBSD.org) Date: Wed, 23 Oct 2013 12:12:37 +0200 From: Tijl Coosemans To: Jilles Tjoelker Subject: Re: VLC 2.1.0 Message-ID: <20131023121237.51e5a79e@kalimero.tijl.coosemans.org> In-Reply-To: <20131022212327.GB20055@stack.nl> References: <20131022152502.61214646@kalimero.tijl.coosemans.org> <20131022174715.59433270@kalimero.tijl.coosemans.org> <20131022212327.GB20055@stack.nl> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="MP_/pNkYODNbZBn4WVupizbFP3L" Cc: multimedia-list freebsd , William Grzybowski , freebsd-standards@FreeBSD.org X-BeenThere: freebsd-standards@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Standards compliance List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 23 Oct 2013 10:12:42 -0000 --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--