Re: [Qemu-devel] [PATCH] Added NULL check for qemu_find_file()

[Stefan Hajnoczi]

On Sun, Mar 13, 2016 at 02:06:34AM +0530, address@hidden wrote:
> From: Rutuja Shah <address@hidden>
> 
> This patch adds NULL check for return value from qemu_find_file(), where it 
> is missing. It avoids unnecessary function calls with NULL parameter which in 
> turn return -1. Especially, incase of load_uimage(), two functions are called 
> which return -1 on passing NULL filename.

What is the benefit of adding the NULL checks?  The only difference I
see is that the error message from load_elf() is silenced.

Avoiding unnecessary function calls isn't worthwhile if all callers now
duplicate the if (filename) ... else size = -1 code.

Please wrap the commit description (the body of the commit message) to
72 characters or at most 80 characters.

Please include your Signed-off-by line here:
http://qemu-project.org/Contribute/SubmitAPatch#Patch_emails_must_include_a_Signed-off-by:_line

> ---
>  hw/ppc/e500.c    | 17 +++++++++++++----
>  hw/sparc/leon3.c |  6 +++++-
>  2 files changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
> index 09154fa..006adf1 100644
> --- a/hw/ppc/e500.c
> +++ b/hw/ppc/e500.c
> @@ -1016,15 +1016,24 @@ void ppce500_init(MachineState *machine, 
> PPCE500Params *params)
>      }
>      filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
>  
> -    bios_size = load_elf(filename, NULL, NULL, &bios_entry, &loadaddr, NULL,
> -                         1, PPC_ELF_MACHINE, 0, 0);
> +    if (filename) {
> +        bios_size = load_elf(filename, NULL, NULL, &bios_entry, &loadaddr, 
> NULL,
> +                             1, PPC_ELF_MACHINE, 0, 0);
> +    } else {
> +        bios_size = -1;
> +    }
> +
>      if (bios_size < 0) {
>          /*
>           * Hrm. No ELF image? Try a uImage, maybe someone is giving us an
>           * ePAPR compliant kernel
>           */
> -        kernel_size = load_uimage(filename, &bios_entry, &loadaddr, NULL,
> -                                  NULL, NULL);
> +        if (filename) {
> +            kernel_size = load_uimage(filename, &bios_entry, &loadaddr, NULL,
> +                                      NULL, NULL);
> +        } else {
> +            kernel_size = -1;
> +        }
>          if (kernel_size < 0) {
>              fprintf(stderr, "qemu: could not load firmware '%s'\n", 
> filename);
>              exit(1);
> diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
> index c579f5b..b4e9334 100644
> --- a/hw/sparc/leon3.c
> +++ b/hw/sparc/leon3.c
> @@ -168,7 +168,11 @@ static void leon3_generic_hw_init(MachineState *machine)
>      }
>      filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
>  
> -    bios_size = get_image_size(filename);
> +    if (filename) {
> +        bios_size = get_image_size(filename);
> +    } else {
> +        bios_size = -1;
> +    }
>  
>      if (bios_size > prom_size) {
>          fprintf(stderr, "qemu: could not load prom '%s': file too big\n",
> 
> 1.9.1
> 
> Signed-off-by: Rutuja Shah <address@hidden>
> 
> Regards
> Rutuja Shah
> 
>

signature.asc
Description: PGP signature