From owner-freebsd-arch@FreeBSD.ORG Tue May 28 05:06:30 2013 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id AD5926CD; Tue, 28 May 2013 05:06:30 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from fallbackmx07.syd.optusnet.com.au (fallbackmx07.syd.optusnet.com.au [211.29.132.9]) by mx1.freebsd.org (Postfix) with ESMTP id 4BA2F859; Tue, 28 May 2013 05:06:29 +0000 (UTC) Received: from mail09.syd.optusnet.com.au (mail09.syd.optusnet.com.au [211.29.132.190]) by fallbackmx07.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id r4S56JkF012684; Tue, 28 May 2013 15:06:19 +1000 Received: from c122-106-156-23.carlnfd1.nsw.optusnet.com.au (c122-106-156-23.carlnfd1.nsw.optusnet.com.au [122.106.156.23]) by mail09.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id r4S55oF8030831 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 28 May 2013 15:06:08 +1000 Date: Tue, 28 May 2013 15:05:50 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Alfred Perlstein Subject: Re: [RFC] add NetBSD compat macros to sys/cdefs.h In-Reply-To: <51A43176.9060100@mu.org> Message-ID: <20130528143646.L1298@besplex.bde.org> References: <51A43176.9060100@mu.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.0 cv=e4Ne0tV/ c=1 sm=1 a=SRymRyOzv8wA:10 a=kj9zAlcOel0A:10 a=PO7r1zJSAAAA:8 a=JzwRw_2MAAAA:8 a=-JRSxk00JnkA:10 a=2Yps3Zx4pix0iROAEYgA:9 a=CjuIK1q_8ugA:10 a=ebeQFi2P/qHVC0Yw9JDJ4g==:117 Cc: Brooks Davis , Garrett Cooper , freebsd-arch@freebsd.org X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 28 May 2013 05:06:30 -0000 On Mon, 27 May 2013, Alfred Perlstein wrote: > On 5/27/13 7:21 PM, Garrett Cooper wrote: >> Hi, >> One of the things that I've done in order to reduce unnecessary >> divergence when porting NetBSD testcases to FreeBSD is I've pushed a >> few macros into my sys/cdefs.h in order to facilitate compatibility >> with NetBSD: >> >> /* NetBSD compat */ >> /* >> * The following macro is used to remove const cast-away warnings >> * from gcc -Wcast-qual; it should be used with caution because it >> * can hide valid errors; in particular most valid uses are in >> * situations where the API requires it, not to cast away string >> * constants. We don't use *intptr_t on purpose here and we are >> * explicit about unsigned long so that we don't have additional >> * dependencies. >> */ >> #define __UNCONST(a) ((void *)(unsigned long)(const void *)(a)) Ugh. Like __DECONST(), this shouldn't exist. It never existed in my versions of FreeBSD. I policed its use a bit until FreeBSD-5, so that my local patches for FreeBSD-5 only have to burn it in 8 files (3 in the kernel and 5 in the kernel). Warnings from -Wcast-qual should never be "fixed" using this. Either don't use -Wcast-qual, or fix the bugs that it finds. Sometimes it is impossible to fix the bugs because they are in a standard API. Then all methods of avoiding the warning are bad, and using __DECONST() is one of the worst. Its only advantage is that it is so ugly and unportable that it serves to document the API bug. Unlike __DECONST(), __UNCONST() doesn't even work. uintptr_t (better __uintptr_t to reduce dependencies) must be used, since unsigned long just doesn't work unless it is identical to uintptr_t and that is true on all 32-bit arches. Apart from its design errors, the __DEMISTAKE() family has many implementation errors. I only fixed these by removing it. It is large and ugly, but not large and ugly enough to work in all cases. The main broken cases are: - __DECONST() on a const volatile type (it strips the volatile qualifier) - __DEVOLATILE() on a const volatile type (it strips the const qualifier) - missing casts from uintptr_t to void * on the way back to a pointer. uintptr_t is only guaranteed to work with void * or qualified void *. It is a feature that it is not documented in any man page. This should prevent its use. Its dependency on other headers is OK since this would be in the documentation if it had any. >> >> #define ___STRING(x) __STRING(x) >> #define __arraycount(__x) (sizeof(__x) / sizeof(__x[0])) >> /* End NetBSD compat */ >> >> To clarify... >> - I know __UNCONST is basically like __DECONST on steroids as >> __UNCONST doesn't have the typecasting like __DECONST does. It is not really on steroids, just has different broken casts. It isn't missing the cast to void * on the way back to a pointer. Not casting to a final pointer type is probably better, but gives subtly different semantics. Bruce