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

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1162 bytes --]

On Mon, Mar 14, 2016 at 06:29:06PM +0530, rutuja shah wrote:
> > What is the benefit of adding the NULL checks?  The only difference I
> > see is that the error message from load_elf() is silenced.
> Yes, in case of load_elf, error message is silenced. The immediate
> call to load_uimage()
> is conditional to error from load_elf(). load_uimage() in turn calls
> load_uboot_image() which
> then calls open() with NULL filename and returns with -1. In my
> opinion, this can be avoided.
> >
> > Avoiding unnecessary function calls isn't worthwhile if all callers now
> > duplicate the if (filename) ... else size = -1 code.
> As far as I have seen the code, all callers have this check in place
> already, except the patched files.

Does this mean the main purpose of the patch is to make these
load_elf()/load_uimage() callers consistent with other callers?

The commit description should say "why" rather than "what" the patch is
doing.

In it's current state I'm not sure what the benefit of this change is.
I also wonder whether the load_elf()/load_uimage() function prototype
should be adjusted to avoid the boilerplate if statement in all callers.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]