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@xxxxxxxxxx>
>>> ---
>>>
>>> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list