qemu-trivial
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-trivial] [Qemu-devel] [PATCH 1/3] tests/boot-sector: Use minim


From: Eric Blake
Subject: Re: [Qemu-trivial] [Qemu-devel] [PATCH 1/3] tests/boot-sector: Use minimum length for the Forth boot script
Date: Tue, 11 Oct 2016 10:58:33 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0

On 10/11/2016 10:19 AM, Thomas Huth wrote:
> The pxe-test is quite slow on ppc64 with tcg. We can speed it up
> a little bit by decreasing the size of the file that has to be
> loaded via TFTP.
> 
> Signed-off-by: Thomas Huth <address@hidden>
> ---
>  tests/boot-sector.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/boot-sector.c b/tests/boot-sector.c
> index e3193c0..0168fd0 100644
> --- a/tests/boot-sector.c
> +++ b/tests/boot-sector.c
> @@ -72,6 +72,7 @@ static uint8_t boot_sector[0x7e000] = {

The size of 0x7e000...

>  int boot_sector_init(const char *fname)
>  {
>      FILE *f = fopen(fname, "w");
> +    size_t len = sizeof boot_sector;
>  
>      if (!f) {
>          fprintf(stderr, "Couldn't open \"%s\": %s", fname, strerror(errno));
> @@ -80,13 +81,12 @@ int boot_sector_init(const char *fname)
>  
>      /* For Open Firmware based system, we can use a Forth script instead */
>      if (strcmp(qtest_get_arch(), "ppc64") == 0) {
> -        memset(boot_sector, ' ', sizeof boot_sector);
> -        sprintf((char *)boot_sector, "\\ Bootscript\n%x %x c! %x %x c!\n",
> +        len = sprintf((char *)boot_sector, "\\ Bootscript\n%x %x c! %x %x 
> c!\n",

...is enough to ensure the sprintf() doesn't overflow.  Still, I think
an snprintf() would be a bit nicer on maintainers to not have to worry
about whether overflow is even possible.

>                  LOW(SIGNATURE), BOOT_SECTOR_ADDRESS + SIGNATURE_OFFSET,
>                  HIGH(SIGNATURE), BOOT_SECTOR_ADDRESS + SIGNATURE_OFFSET + 1);

Worth reindenting this to match the line above?

>      }
>  
> -    fwrite(boot_sector, 1, sizeof boot_sector, f);
> +    fwrite(boot_sector, 1, len, f);

At any rate, the change makes sense, and I can't find anything
technically wrong with it, so feel free to add:
Reviewed-by: Eric Blake <address@hidden>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]