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(a)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