From owner-svn-src-head@FreeBSD.ORG Fri Sep 24 01:26:46 2010 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 10C3F106564A; Fri, 24 Sep 2010 01:26:46 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail02.syd.optusnet.com.au (mail02.syd.optusnet.com.au [211.29.132.183]) by mx1.freebsd.org (Postfix) with ESMTP id 9B6698FC15; Fri, 24 Sep 2010 01:26:45 +0000 (UTC) Received: from c122-107-116-249.carlnfd1.nsw.optusnet.com.au (c122-107-116-249.carlnfd1.nsw.optusnet.com.au [122.107.116.249]) by mail02.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id o8O1QfjK024882 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 24 Sep 2010 11:26:42 +1000 Date: Fri, 24 Sep 2010 11:26:41 +1000 (EST) From: Bruce Evans X-X-Sender: bde@delplex.bde.org To: Marcel Moolenaar In-Reply-To: <11C42F9D-7201-4804-8923-430F28E58C57@mac.com> Message-ID: <20100924105219.J780@delplex.bde.org> References: <201009200420.o8K4KtKn026514@svn.freebsd.org> <20100920163758.A788@besplex.bde.org> <11C42F9D-7201-4804-8923-430F28E58C57@mac.com> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, Marcel Moolenaar , src-committers@freebsd.org, Bruce Evans Subject: Re: svn commit: r212886 - head/sbin/growfs X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 24 Sep 2010 01:26:46 -0000 On Mon, 20 Sep 2010, Marcel Moolenaar wrote: > On Sep 19, 2010, at 11:55 PM, Bruce Evans wrote: > >> On Mon, 20 Sep 2010, Marcel Moolenaar wrote: >> >>> Log: >>> Unbreak the build on strong-aligned architectures (arm, ia64). >>> Casting from (char *) to (struct ufs1_dinode *) changes the >>> alignment requirement of the pointer and GCC does not know that >>> the pointer is adequately aligned (due to malloc(3)), and warns >>> about it. Cast to (void *) first to by-pass the check. >> >> A previous version in which the pointer (iobuf) the correct type to >> begin with was better. It used to have type void *, but now has type >> caddr_t (whose existence is a bug), in order to abuse caddr_t by >> assuming that it is char * to do pointer arithmetic on it in 1 place >> (iobuf + sblock.fs_cgsize). The 7 other places where iobuf is used >> only assume that caddr_t is a pointer (or perhaps a [u]intptr_t). >> >> growfs has no other instances of caddr_t. > > My first instinct was to retype iobuf as (void *), but saw > the pointer arithmetic and decided against it. If we had > a macro that one can use to add X number of bytes to a pointer > to any, I would have used that. Other macros to add X number > of short, int, long, could be handy too, but that may not be > as useful as for bytes. > > Would it be useful to add a macro like (don't mind the name): > > #define ADDBYTESTOPOINTER(ptr, cnt) \ > ((__typeof(ptr))(void *)((uintptr_t)(void *)(ptr) + (cnt))) As you may know, I like such macros in system headers as much as I like __DECONST() -- they are good enough for lining /dev/null but only after suitable heating :-). They are unportable and make it too easy to use wrong types. They are OK in individual applications, but most applications should have few instances of pointer frobbing so they don't need macros for it except to highlight its bogusness. The above implementation has the technical error of casting away any qualifiers of (ptr). __DECONST() is OK in that respect (it is good for it to be an error if it removes more than a const qualifier, but it has the technical error of not casting uintptr_t to (void *). If you don't bother with the cast to (void *), then you could also not bother with the cast to (const void *), except the latter has the advantage of detecting removal of more than a const qualifier -- perhaps that was intentional. Both depend on the bug that -Wcast-qual doesn't work for casting to uintptr_t. I think I prefer casting to (char *) than to uintptr_t for adding byte offsets to pointers. These give different problems with casting away qualifiers, as mentioned above. Perhaps the best way is application- dependent. But portable applications cannot use uintptr_t for this except in negatively useful ifdefs, since uintptr_t might not exist. IMO, the correct fix for fixing -Wcast-qual errors is to not use -Wcast-qual. The Standard method for removing type qualifiers is to explicitly cast them away, and -Wcast-qual breaks this. But bogus casts are still too common, so first you have to find them all. There should be a -Wbogus-cast option to help find them. This is a DWIM option so I don't know how to implement it :-). It could start by warning about casts that are null on all target, then warn about casts that are null on the current target. There should remain a cast-qual option that only applies to implicit casts (for assignments and function calls). Bruce