On 5/21/26 14:22, Lucas Kornicki wrote:
On 5/21/26 10:39, Michal Prívozník wrote:
So you'd want to drop the `if (events[0])` and leave just the `else`? Or drop the `else` altogether? I see no harm in keeping it this way, but I'll defer to your judgement. I'd just keep queuing both events and drop else branch. Even in the first hunk (or pre-existing code) there's no NULL check. It's simply:
event = virDomainEventXXXNewFromObj(...); virObjectEventStateQueue(..., event);
And all of virDomainEventXXXNewFromObj() can fail and return NULL.
Michal
Ah I think I remember why I've done it this way. If we just queue events[1] and events[0] there is a possibility that we only end up sending the agent event if construction of channel event fails. I don't think this is much of a practical concern as if NewFromObj fails, it will likely fail for both events. At least with explicit control flow it will be evident what the intention was.
If we don't care about this sort of consistency we can rely on NOP queuing behavior. Thoughts?
Yeah, we already rely on NOP behavior so let's keep the pattern.
Do you want me to send in a v3, or will you apply the changes when merging?
Nah, I'll change it before merging. Michal