[Top][All Lists]

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

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

From: rutuja shah
Subject: Re: [Qemu-devel] [PATCH] Added NULL check for qemu_find_file()
Date: Tue, 15 Mar 2016 18:26:05 +0530

Rutuja Shah

On Tue, Mar 15, 2016 at 6:01 PM, Stefan Hajnoczi <address@hidden> wrote:
> 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?
This is a task listed on http://qemu-project.org/BiteSizedTasks
(presently, I am unaware of the intention it is put up here). I
propose this change so that callers are consistent. Also it adds to
the performance, if such a case is encountered for reasons mentioned
> 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.
Changing the prototype would result in a cleaner code, for all the
functions which are using the file name received from
> Stefan

reply via email to

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