Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 13 Aug 2010 19:06:48 +1000 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        Takanori Watanabe <takawata@FreeBSD.org>
Cc:        svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org
Subject:   Re: svn commit: r211252 - head/usr.sbin/acpi/acpidump
Message-ID:  <20100813190247.I12761@delplex.bde.org>
In-Reply-To: <201008130045.o7D0jUW6043601@svn.freebsd.org>
References:  <201008130045.o7D0jUW6043601@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 13 Aug 2010, Takanori Watanabe wrote:

> Log:
>  Fix build on amd64 and ia64.

Why not fix it on all arches?

> Modified: head/usr.sbin/acpi/acpidump/acpi.c
> ==============================================================================
> --- head/usr.sbin/acpi/acpidump/acpi.c	Fri Aug 13 00:21:32 2010	(r211251)
> +++ head/usr.sbin/acpi/acpidump/acpi.c	Fri Aug 13 00:45:30 2010	(r211252)
> ...
> @@ -623,7 +622,7 @@ acpi_handle_tcpa(ACPI_TABLE_HEADER *sdp)
> {
> 	struct TCPAbody *tcpa;
> 	struct TCPAevent *event;
> -	u_int64_t len, paddr;
> +	uint64_t len, paddr;
> 	unsigned char *vaddr = NULL;
> 	unsigned char *vend = NULL;
>
> @@ -647,7 +646,7 @@ acpi_handle_tcpa(ACPI_TABLE_HEADER *sdp)
> 		printf(END_COMMENT);
> 		return;
> 	}
> -	printf("\tClass %d Base Address 0x%jx Length %" PRIu64 "\n\n",
> +	printf("\tClass %u Base Address 0x%jx Length %ju\n\n",
> 	    tcpa->platform_class, paddr, len);
>
> 	if (len == 0) {

For `len', this used to assume that variables of type u_int64_t can
be printed using PRIu64.  Why the PRIu64 abomination should never be
used, this assumption is valid.

For `len', this now assumes that variables of type uint64_t can be
printed using %ju format.  %ju format is for printing variables of
type uintmax_t, so this assumption is invalid unless uint64_t is the
same as uintmax_t.  This assumption happens to be valid on all supported
arches, although it should not be (e.g., on amd64, uint64_t and uintmax_t
both happen to be u_long, but this is illogical since uintmax_t is
supposed to be the largest unsigned integer type but u_long is logically
shorter than unsigned long long).  Fixing these arches might expose
many printf format errors like the above.

For `paddr', this used to and still invalidly assumes that variables of
type uint64_t can be printed using %ju format.

PRIu64 is "lu" on both amd64 and ia64, and __uint64_t is u_long on both
amd64 and ia64, so I don't see how the original version failed.  In fact,
it doesn't fail for me.

__uintmax_t is u_long on both amd64 and ia64, so the modified version
should work too, though accidentally, just like the unmodified version
works accidentally for `paddr'.

To expose even more printf format errors like the above, make __uintmax_t
unsigned long long on amd64 and keep it as the illogical u_long on amd64.

Bruce



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