From owner-svn-src-head@FreeBSD.ORG Sun Oct 27 17:42:46 2013 Return-Path: Delivered-To: svn-src-head@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 F2A98565; Sun, 27 Oct 2013 17:42:45 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail110.syd.optusnet.com.au (mail110.syd.optusnet.com.au [211.29.132.97]) by mx1.freebsd.org (Postfix) with ESMTP id B4AF822FC; Sun, 27 Oct 2013 17:42:45 +0000 (UTC) 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 mail110.syd.optusnet.com.au (Postfix) with ESMTPS id D2DE9781274; Mon, 28 Oct 2013 04:42:41 +1100 (EST) Date: Mon, 28 Oct 2013 04:42:41 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Ian Lepore Subject: Re: svn commit: r257203 - head/sys/arm/arm In-Reply-To: <201310270329.r9R3Tcoi076809@svn.freebsd.org> Message-ID: <20131028040222.G929@besplex.bde.org> References: <201310270329.r9R3Tcoi076809@svn.freebsd.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.1 cv=YYGEuWhf c=1 sm=1 tr=0 a=ebeQFi2P/qHVC0Yw9JDJ4g==:117 a=PO7r1zJSAAAA:8 a=zbkiI427TaUA:10 a=kj9zAlcOel0A:10 a=JzwRw_2MAAAA:8 a=E11o_6pwCccA:10 a=y8rJQ-hqLJ560y21m9wA:9 a=CjuIK1q_8ugA:10 Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.14 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: Sun, 27 Oct 2013 17:42:46 -0000 On Sun, 27 Oct 2013, Ian Lepore wrote: > Log: > Eliminate a compiler warning about extraneous parens. Wow, what flags give these warnings? Next I'll ask for -Wexcessive-braces, -Wbogus-cast and -Wno-sloppy-format. > Modified: head/sys/arm/arm/busdma_machdep.c > ============================================================================== > --- head/sys/arm/arm/busdma_machdep.c Sun Oct 27 03:24:46 2013 (r257202) > +++ head/sys/arm/arm/busdma_machdep.c Sun Oct 27 03:29:38 2013 (r257203) > @@ -807,7 +807,7 @@ _bus_dmamap_count_phys(bus_dma_tag_t dma > bus_addr_t curaddr; > bus_size_t sgsize; > > - if ((map->pagesneeded == 0)) { > + if (map->pagesneeded == 0) { > CTR3(KTR_BUSDMA, "lowaddr= %d, boundary= %d, alignment= %d", > dmat->lowaddr, dmat->boundary, dmat->alignment); > CTR2(KTR_BUSDMA, "map= %p, pagesneeded= %d", Formats for CTR*s are extremely bogus and can't be handled by -Wformat or -Wno-sloppy-format. If it did, then -Wno-sloppy-format would complain about printing unsigned longs (bus_addr_t's) using %d format even if -Wformat wouldn't because unsigned longs have the same size as ints. CTR*() is actually not variadic, but converts all args to u_long, so %d and %p cannot work without further magic to convert back to the original type. I never been able to find this magic and suspect it doesn't exist. I think the format strings are not used in the kernel. They are used with gross type mismatches in ktrdump unless all the format specifiers in them are %lu or similar: ktrdump just does: fprintf(out, desc, parms[0], parms[1], parms[2], parms[3], parms[4], parms[5]); It parses the format string using a primitive parser but doesn't rewrite any of the types or args except to replace kernel pointers to strings by user pointers to copies of the strings. Integer and pointer args remain in parms[] as u_longs from the kernel conversion, and are then printed with the format specifiers in the CTR*() calls which almost never match the u_long. Most of the kernel conversions to u_long are automatic, but sometimes bogus casts (which might match neither the format specifier nor u_long) are used to defeat the type checking in automatic conversions. If the original types are 64 bits, then passing them as u_long's just doesn't work and some CTR*() calls use extremely ugly code with bogus casts in it to split the 64 into two. The undefined behaviour in ktrdump can be fixed fairly easily by rewriting all the integer format specifiers to %lu or %ld. %p needs to be left as itself since %p format is too different from %lu format. But without format checking, the formats will often be too broken to be displayed correctly. %d in the above is a bad format for addresses anyway. %jx is always wrong in MI code -- it works if u_long == uintmax_t, but on 32-bit arches with large values in the original type uintmax_t, it indicates that the programmer doesn't know that CTR*() will truncate to u_long. Bruce