Re: svn commit: r211221 - head/usr.sbin/acpi/acpidump
On Thu, 12 Aug 2010, John Baldwin wrote: Dag-Erling Sm??rgrav wrote: John Baldwin j...@freebsd.org writes: Dag-Erling Sm??rgrav d...@des.no writes: Oops, I replied to the commit mail for the round after this before noticing this thread. Slightly better: printf(\tClass %u Base Address 0x%jx Length %ju\n\n, (unsigned int)tcpa-platform_class, (uintmax_t)paddr, (uintmax_t)len); Ugh, this has many more style bugs: - `unsigned int' is spelled u_int in KNF, partly to try to avoid the next bug - `platform_class' has type uint16_t. u_int is larger than that on all supported machines, and also on unsupported ones with 16-31 bit u_ints. Thus the cast has almost no effect, and has no effect on the result. On unsupported machines with exactly 16-bit u_ints, it has no effect. On unsupported and supported machines with = 17-bit ints, the arg gets promoted to a signed int thanks to C90's promotion botch, but the value of the signed int is nonnegative so there is no difference here from the result of an unbotched promotion to u_int. - the second line is too ling, and has been nicely mangled by mailers, first by putting a hard newline in it and then by quoting it. but it would probably be easier to define paddr and len as unsigned long long instead of the misspelled u_int64_t, and use %llx and %llu. Depends. If the table defines a field to be a 64-bit integer, it is better to use an explicitly-64-bit integer type such as uint64_t rather than assuming that 'long long' is 64-bit. Nah, unsigned long long shouldn't exist. Actually, paddr and len are a memory address and an object size, respectively, so the logical thing would be to use uintptr_t and size_t with uintmax_t casts... vm_offset_t and vm_size_t would be more logical, but maybe they are too BSD(Mach vm)-specific for acpi, especially outside of the kernel. Except that physical addresses do not always fit in uintptr_t on i386 (think PAE with 36-bit physical addresses and 32-bit uintptr_t, same for the length). If they truly were a size_t you could use %z without a uintmax_t cast. Yes, if paddr really means a physical and not a virtual memory address. PAE has vm_paddr_t for the former. vm_paddr_t is 32 bits without PAE and 64 bits with PAE. Printing it and otherwise using it gives the usual problem that _no_ typedefed type can be printed or otherwise used with a hard-coded format or type (other than the typedef), but bugs are more noticable than usual since the type varies widely within a single arch. Anyway, almost all typedefed types should be cast to [u]intmax_t for printing, so that you don't have to know too much about what they are. We are still very sloppy about this for the smaller types, but have to be more careful for 64-bit types. Sloppy printing of the smaller types will break eventually when int or long expands but the typedefs don't. Bruce___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
Re: svn commit: r211221 - head/usr.sbin/acpi/acpidump
Bruce Evans b...@optusnet.com.au writes: - `platform_class' has type uint16_t. u_int is larger than that on all supported machines, and also on unsupported ones with 16-31 bit u_ints. Thus the cast has almost no effect, and has no effect on the result. you have to cast it to *something*, unless you're willing to assume blindly that uint16_t == unsigned short (and use %h). Anyway, almost all typedefed types should be cast to [u]intmax_t for printing, so that you don't have to know too much about what they are. long or even int is fine in many cases, e.g. uid_t, mode_t DES -- Dag-Erling Smørgrav - d...@des.no ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
Re: svn commit: r211221 - head/usr.sbin/acpi/acpidump
On Fri, 13 Aug 2010, [utf-8] Dag-Erling Sm??rgrav wrote: Bruce Evans b...@optusnet.com.au writes: - `platform_class' has type uint16_t. u_int is larger than that on all supported machines, and also on unsupported ones with 16-31 bit u_ints. Thus the cast has almost no effect, and has no effect on the result. you have to cast it to *something*, unless you're willing to assume blindly that uint16_t == unsigned short (and use %h). No, all function parameters are converted to *something*, and as I partly explained above, the default something is always int or u_int, with the lofical sign error for the int not causing any problems. Various cases: - KR compiler, or C90-C99 compiler with no prototype in scope: integer parameters are converted to int or u_int if they are smaller than that to start, else they are not converted. - variadic function for a parameter after `...': same as for KR. - %h format for printf: makes little or no difference except for bogus parameters. The parameter must end up as int or u_int after the default promotion. The %h causes the parameter to be downcast to short or u_short. This is different from taking a short or u_short parameter. You just can't pass a short or a u_short to a variadic function, Here uint16_t is converted to u_int on machines with 16-bit ints, else it is converted to int. Anyway, almost all typedefed types should be cast to [u]intmax_t for printing, so that you don't have to know too much about what they are. long or even int is fine in many cases, e.g. uid_t, mode_t Though it works now, it might break someday. 32-bit ino_t and printing it with %u should have broken already. Bruce___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
svn commit: r211221 - head/usr.sbin/acpi/acpidump
Author: takawata
Date: Thu Aug 12 13:58:46 2010
New Revision: 211221
URL: http://svn.freebsd.org/changeset/base/211221
Log:
Fix breakage on 64bit architecture by using inttypes.h macro.
Modified:
head/usr.sbin/acpi/acpidump/acpi.c
Modified: head/usr.sbin/acpi/acpidump/acpi.c
==
--- head/usr.sbin/acpi/acpidump/acpi.c Thu Aug 12 13:46:43 2010
(r211220)
+++ head/usr.sbin/acpi/acpidump/acpi.c Thu Aug 12 13:58:46 2010
(r211221)
@@ -40,6 +40,7 @@
#include stdlib.h
#include string.h
#include unistd.h
+#include inttypes.h
#include acpidump.h
@@ -646,7 +647,7 @@ acpi_handle_tcpa(ACPI_TABLE_HEADER *sdp)
printf(END_COMMENT);
return;
}
- printf(\tClass %d Base Address 0x%jx Length %llu\n\n,
+ printf(\tClass %d Base Address 0x%jx Length % PRIu64 \n\n,
tcpa-platform_class, paddr, len);
if (len == 0) {
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
Re: svn commit: r211221 - head/usr.sbin/acpi/acpidump
Takanori Watanabe takaw...@freebsd.org writes: - printf(\tClass %d Base Address 0x%jx Length %llu\n\n, + printf(\tClass %d Base Address 0x%jx Length % PRIu64 \n\n, tcpa-platform_class, paddr, len); This is just as wrong as the previous attempt. 1) platform_class is not an int. 2) paddr is not a uintmax_t. 3) so far, we've avoided using the PRI macros. Slightly better: printf(\tClass %u Base Address 0x%jx Length %ju\n\n, (unsigned int)tcpa-platform_class, (uintmax_t)paddr, (uintmax_t)len); but it would probably be easier to define paddr and len as unsigned long long instead of the misspelled u_int64_t, and use %llx and %llu. DES -- Dag-Erling Smørgrav - d...@des.no ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
Re: svn commit: r211221 - head/usr.sbin/acpi/acpidump
Dag-Erling Smørgrav wrote: Takanori Watanabe takaw...@freebsd.org writes: - printf(\tClass %d Base Address 0x%jx Length %llu\n\n, + printf(\tClass %d Base Address 0x%jx Length % PRIu64 \n\n, tcpa-platform_class, paddr, len); This is just as wrong as the previous attempt. 1) platform_class is not an int. 2) paddr is not a uintmax_t. 3) so far, we've avoided using the PRI macros. Slightly better: printf(\tClass %u Base Address 0x%jx Length %ju\n\n, (unsigned int)tcpa-platform_class, (uintmax_t)paddr, (uintmax_t)len); but it would probably be easier to define paddr and len as unsigned long long instead of the misspelled u_int64_t, and use %llx and %llu. Depends. If the table defines a field to be a 64-bit integer, it is better to use an explicitly-64-bit integer type such as uint64_t rather than assuming that 'long long' is 64-bit. Other ACPI table definitions all use fixed-size types like uint32_t and uint64_t since the tables are defined as fixed-size fields, not as 'long' and 'int'. Using %j with uintmax_t casts is the solution used for other 64-bit fields in ACPI tables. -- John Baldwin ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
Re: svn commit: r211221 - head/usr.sbin/acpi/acpidump
John Baldwin j...@freebsd.org writes: Dag-Erling Smørgrav d...@des.no writes: Slightly better: printf(\tClass %u Base Address 0x%jx Length %ju\n\n, (unsigned int)tcpa-platform_class, (uintmax_t)paddr, (uintmax_t)len); but it would probably be easier to define paddr and len as unsigned long long instead of the misspelled u_int64_t, and use %llx and %llu. Depends. If the table defines a field to be a 64-bit integer, it is better to use an explicitly-64-bit integer type such as uint64_t rather than assuming that 'long long' is 64-bit. Actually, paddr and len are a memory address and an object size, respectively, so the logical thing would be to use uintptr_t and size_t with uintmax_t casts... DES -- Dag-Erling Smørgrav - d...@des.no ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
Re: svn commit: r211221 - head/usr.sbin/acpi/acpidump
Dag-Erling Smørgrav wrote: John Baldwin j...@freebsd.org writes: Dag-Erling Smørgrav d...@des.no writes: Slightly better: printf(\tClass %u Base Address 0x%jx Length %ju\n\n, (unsigned int)tcpa-platform_class, (uintmax_t)paddr, (uintmax_t)len); but it would probably be easier to define paddr and len as unsigned long long instead of the misspelled u_int64_t, and use %llx and %llu. Depends. If the table defines a field to be a 64-bit integer, it is better to use an explicitly-64-bit integer type such as uint64_t rather than assuming that 'long long' is 64-bit. Actually, paddr and len are a memory address and an object size, respectively, so the logical thing would be to use uintptr_t and size_t with uintmax_t casts... Except that physical addresses do not always fit in uintptr_t on i386 (think PAE with 36-bit physical addresses and 32-bit uintptr_t, same for the length). If they truly were a size_t you could use %z without a uintmax_t cast. -- John Baldwin ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
