On Sun, Mar 13, 2016 at 02:06:34AM +0530, rutu.shah...@gmail.com wrote: > From: Rutuja Shah <rutu.shah...@gmail.com> > > 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 <rutu.shah...@gmail.com> > > Regards > Rutuja Shah > >
signature.asc
Description: PGP signature