Re: svn commit: r281307 - head/sys/boot/efi/boot1
On Thu, 9 Apr 2015 21:38:02 +1000 (EST) Bruce Evans b...@optusnet.com.au wrote: On Thu, 9 Apr 2015, Andrew Turner wrote: Log: Print error values with hex to make it easier to find the EFI error type. Modified: head/sys/boot/efi/boot1/boot1.c Modified: head/sys/boot/efi/boot1/boot1.c == --- head/sys/boot/efi/boot1/boot1.c Thu Apr 9 10:12:58 2015(r281306) +++ head/sys/boot/efi/boot1/boot1.c Thu Apr 9 10:15:47 2015(r281307) @@ -330,18 +330,18 @@ load(const char *fname) status = systab-BootServices-LoadImage(TRUE, image, bootdevpath, buffer, bufsize, loaderhandle); if (EFI_ERROR(status)) - printf(LoadImage failed with error %d\n, status); + printf(LoadImage failed with error %lx\n, status); How would anyone guess that a number like 10 is in hex? Hex numbers should usually be printed using %#... format. If the boot loader doesn't have that, then use an 0x prefix. This shouldn't compile. 'status' cannot have type int and type unsigned long at the same time. clang warns even without -Wformat in CFLAGS. It is either uint32_t on 32-bit architectures, or uint64_t on 64-bit architectures. I know it's wrong on 32-bit, however on both architectures we use this code a long is 32-bit. Andrew ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r281307 - head/sys/boot/efi/boot1
On Mon, 13 Apr 2015, Andrew Turner wrote: On Thu, 9 Apr 2015 21:38:02 +1000 (EST) Bruce Evans b...@optusnet.com.au wrote: On Thu, 9 Apr 2015, Andrew Turner wrote: Log: Print error values with hex to make it easier to find the EFI error type. Modified: head/sys/boot/efi/boot1/boot1.c Modified: head/sys/boot/efi/boot1/boot1.c == --- head/sys/boot/efi/boot1/boot1.c Thu Apr 9 10:12:58 2015(r281306) +++ head/sys/boot/efi/boot1/boot1.c Thu Apr 9 10:15:47 2015(r281307) @@ -330,18 +330,18 @@ load(const char *fname) status = systab-BootServices-LoadImage(TRUE, image, bootdevpath, buffer, bufsize, loaderhandle); if (EFI_ERROR(status)) - printf(LoadImage failed with error %d\n, status); + printf(LoadImage failed with error %lx\n, status); How would anyone guess that a number like 10 is in hex? Hex numbers should usually be printed using %#... format. If the boot loader doesn't have that, then use an 0x prefix. This shouldn't compile. 'status' cannot have type int and type unsigned long at the same time. clang warns even without -Wformat in CFLAGS. It is either uint32_t on 32-bit architectures, or uint64_t on 64-bit architectures. I know it's wrong on 32-bit, however on both architectures we use this code a long is 32-bit. That allows it to run, but not to compile. It only compiles because format checking is broken. Format checking is broken because printf() is not declared as __printflike(). Normally, this doesn't matter, because the compiler knows that printf() is like itself. However, -ffreestanding makes printf() just another function so the compiler must not know anything special about it unless you tell it. The kernel is careful about this and declares printf() as __printflike() in sys/systm.h. Boot programs are not careful about this. Even stdio.h is doesn't declare printf() as like itself. This works provided stdio.h is not (ab)used for the -ffreestanding case. stdio.h declares functions as __printflike() more or less iff they were not in C90. For example, it declares snprintf() as __printflike() since this was necessary before C99 standardized it and is still necessary to support -std=N where N is older than c99. Boot programs are also not careful about __restrict. stdio.h is careful about this. It seems to be careful in the same way as for __printflike(), but that means using it for old functions like printf() too, since it was never automatic in C90. boot1.c actually gets its buggy declaration of printf() by having its own static function named printf() and not declaring this as __printflike(). Similarly for all of its other printflike functions. Similarly in many other boot programs. Grepping for ^printf finds 7 more instances of the bug in 7 different files. Most seem to be the result of using cut and paste to copy the bug from i386/boot2. There is not a single instance of __printflike() in the 8 files. There are 2 instances of in the whole boot hierarchy. Boot headers aren't so broken. stand.h uses __printflike() but hasn't caught up with 'restrict' yet. libstand has a reduced printf() but this is still too bloated to use in the small boot1 and boot2 programs. Bruce ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r281307 - head/sys/boot/efi/boot1
On Thu, 9 Apr 2015, Andrew Turner wrote: Log: Print error values with hex to make it easier to find the EFI error type. Modified: head/sys/boot/efi/boot1/boot1.c Modified: head/sys/boot/efi/boot1/boot1.c == --- head/sys/boot/efi/boot1/boot1.c Thu Apr 9 10:12:58 2015 (r281306) +++ head/sys/boot/efi/boot1/boot1.c Thu Apr 9 10:15:47 2015 (r281307) @@ -330,18 +330,18 @@ load(const char *fname) status = systab-BootServices-LoadImage(TRUE, image, bootdevpath, buffer, bufsize, loaderhandle); if (EFI_ERROR(status)) - printf(LoadImage failed with error %d\n, status); + printf(LoadImage failed with error %lx\n, status); How would anyone guess that a number like 10 is in hex? Hex numbers should usually be printed using %#... format. If the boot loader doesn't have that, then use an 0x prefix. This shouldn't compile. 'status' cannot have type int and type unsigned long at the same time. clang warns even without -Wformat in CFLAGS. Bruce ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
svn commit: r281307 - head/sys/boot/efi/boot1
Author: andrew Date: Thu Apr 9 10:15:47 2015 New Revision: 281307 URL: https://svnweb.freebsd.org/changeset/base/281307 Log: Print error values with hex to make it easier to find the EFI error type. Modified: head/sys/boot/efi/boot1/boot1.c Modified: head/sys/boot/efi/boot1/boot1.c == --- head/sys/boot/efi/boot1/boot1.c Thu Apr 9 10:12:58 2015 (r281306) +++ head/sys/boot/efi/boot1/boot1.c Thu Apr 9 10:15:47 2015 (r281307) @@ -330,18 +330,18 @@ load(const char *fname) status = systab-BootServices-LoadImage(TRUE, image, bootdevpath, buffer, bufsize, loaderhandle); if (EFI_ERROR(status)) - printf(LoadImage failed with error %d\n, status); + printf(LoadImage failed with error %lx\n, status); status = systab-BootServices-HandleProtocol(loaderhandle, LoadedImageGUID, (VOID**)loaded_image); if (EFI_ERROR(status)) - printf(HandleProtocol failed with error %d\n, status); + printf(HandleProtocol failed with error %lx\n, status); loaded_image-DeviceHandle = bootdevhandle; status = systab-BootServices-StartImage(loaderhandle, NULL, NULL); if (EFI_ERROR(status)) - printf(StartImage failed with error %d\n, status); + printf(StartImage failed with error %lx\n, status); } static void ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
