[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] Added NULL check for qemu_find_file()
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH] Added NULL check for qemu_find_file() |
Date: |
Mon, 14 Mar 2016 11:44:13 +0000 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
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
Re: [Qemu-devel] [PATCH] Added NULL check for qemu_find_file(), Eric Blake, 2016/03/14