[libvirt] [PATCH v2 6/8] qemuDomainEventQueue: Check if event is non-NULL

John Ferlan jferlan at redhat.com
Fri Sep 18 12:01:35 UTC 2015



On 09/18/2015 07:46 AM, Jiri Denemark wrote:
> On Thu, Sep 17, 2015 at 18:24:29 -0400, John Ferlan wrote:
>>
>>
>> On 09/11/2015 09:26 AM, Jiri Denemark wrote:
>>> Every single call to qemuDomainEventQueue() uses the following pattern:
>>>
>>>     if (event)
>>>         qemuDomainEventQueue(driver, event);
>>>
>>> Let's move the check for valid event to qemuDomainEventQueue and
>>> simplify all callers.
>>>
>>> Signed-off-by: Jiri Denemark <jdenemar at redhat.com>
>>> ---
>>>
>>> Notes:
>>>     Version 2:
>>>     - no change
>>>
>>>  src/qemu/qemu_blockjob.c  |  6 ++--
>>>  src/qemu/qemu_cgroup.c    |  3 +-
>>>  src/qemu/qemu_domain.c    |  6 ++--
>>>  src/qemu/qemu_driver.c    | 87 ++++++++++++++++-------------------------------
>>>  src/qemu/qemu_hotplug.c   | 26 ++++++--------
>>>  src/qemu/qemu_migration.c | 24 +++++--------
>>>  src/qemu/qemu_process.c   | 72 +++++++++++++--------------------------
>>>  7 files changed, 78 insertions(+), 146 deletions(-)
>>>
>>
>> Yay!  A couple still are behind "if (event) {" in qemu_driver
>> (qemuDomainCreateXML, qemuDomainRevertToSnapshot) as well as an if
> 
> There are two events involved in both functions ("event" and "event2")
> and we can send event2 only if event is not NULL, which is the reason
> for
>     if (event) {
>         qemuDomainEventQueue(driver, event);
>         qemuDomainEventQueue(driver, event2);
>     }
> 
> It would be possible to rewrite it as
> 
>     qemuDomainEventQueue(driver, event);
>     if (event)
>         qemuDomainEventQueue(driver, event2);
> 
> but I think the original version is slightly better.
> 

Right - I did look and went back and forth in my own head a couple of
times, but kept coming back to it seems event2 can only be set if event
is set. Since the new code will ignore NULL - I just figured it was safe
- although I'm perfectly fine with leaving it as you've coded it.

IOW: I saw no path where event2 was set, but event wasn't.

John




More information about the libvir-list mailing list