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
