Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 30 Jun 2012 16:10:29 +1000 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        Brooks Davis <brooks@FreeBSD.org>
Cc:        svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org
Subject:   Re: svn commit: r237820 - head/sys/geom
Message-ID:  <20120630154607.P852@besplex.bde.org>
In-Reply-To: <201206292015.q5TKF0bw068017@svn.freebsd.org>
References:  <201206292015.q5TKF0bw068017@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 29 Jun 2012, Brooks Davis wrote:

> Log:
>  MFP4 #212266
>
>  Fix compile on MIPS64.
>
>  Sponsored by:	DARPA, AFRL
>
> Modified:
>  head/sys/geom/geom_map.c
>
> Modified: head/sys/geom/geom_map.c
> ==============================================================================
> --- head/sys/geom/geom_map.c	Fri Jun 29 19:51:06 2012	(r237819)
> +++ head/sys/geom/geom_map.c	Fri Jun 29 20:15:00 2012	(r237820)
> @@ -119,13 +119,13 @@ g_map_dumpconf(struct sbuf *sb, const ch
> 	g_slice_dumpconf(sb, indent, gp, cp, pp);
> 	if (pp != NULL) {
> 		if (indent == NULL) {
> -			sbuf_printf(sb, " entry %lld", sc->entry[pp->index]);
> -			sbuf_printf(sb, " dsize %lld", sc->dsize[pp->index]);
> +			sbuf_printf(sb, " entry %jd", (intmax_t)sc->entry[pp->index]);
> +			sbuf_printf(sb, " dsize %jd", (intmax_t)sc->dsize[pp->index]);

Style bugs (lines too long).

> @@ -153,8 +153,8 @@ find_marker(struct g_consumer *cp, const
> 		return (1);
>
> 	if (bootverbose) {
> -		printf("MAP: search key \"%s\" from 0x%llx, step 0x%llx\n",
> -		    search_key, search_start, search_step);
> +		printf("MAP: search key \"%s\" from 0x%jx, step 0x%jx\n",
> +		    search_key, (intmax_t)search_start, (intmax_t)search_step);
> 	}

Still has sign mismatches.  %jx takes a uintmax_t arg, but an intmax_t arg
is passed.  The variables start as signed (off_t).  Printing signed values
using hex formats is dubious, but if it is wanted then the conversion
should be done when the parameter is passed instead of depending on
printf() doing something reasonable.

>
> 	/* error if search_key is empty */
> @@ -321,9 +321,10 @@ g_map_parse_part(struct g_class *mp, str
> 	}
>
> 	if (bootverbose) {
> -		printf("MAP: %llxx%llx, data=%llxx%llx "
> +		printf("MAP: %lxx%lx, data=%lxx%lx "
> 		    "\"/dev/map/%s\"\n",
> -		    start, size, offset, dsize, name);
> +		    (intmax_t)start, (intmax_t)size, (intmax_t)offset,
> +		    (intmax_t)dsize, name);

Still has fatal type mimatches which are larger than before:
- before: the format said <expletive deleted> but the default promotion
   of off_t was passed.  Everything was 64 bits in practice, so there
   was no problem at runtime except possibly with the sign mismatch, but
   the static format checker complained
- after: the format says u_long, but intmax_t is passed.  Now the sizes
   are different on arches with 32 bit u_long, and the result is garbage.

Still has sign mismatches, as above.

As well as printf format errors, this still has the following format-printf
errors (formatting style bugs):
- source level: the string is still obfuscated using the C90 string
   concatenation misfeature to split it across multiple lines.  The string
   is not even long enough to not fit on 1 line.
- output level: %lxx%lx is still a weird format.  It consists of 2 hex
   numbers without any 0x prefixes to indicate that they are in hex.
   But there is an "x" with no spaces between the numbers.  This apparently
   means multiplication, but it looks like part of a 0x prefix.

Bruce



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120630154607.P852>