svn commit: r257203 - head/sys/arm/arm

[Bruce Evans]

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