Martin Kletzander wrote:
> Oh yeah, it sure looks like it now. I misrepresented that since having
> goto labels somewhere else than at the top level of the function is a
> bit misleading. The patch applies the way you said and it is correct.
>
> I think I would still rather prefer avoiding the possible error-prone
> style and maybe switch it around a bit. Few ideas:
>
> a) Reverse the condition:
>
> if (kev.filter != EVFILT_PROC || (kev.fflags & NOTE_EXIT) == 0)
> return;
>
> virObjectLock(vm);
> if ((pid_t)kev.ident != vm->pid) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("event from unexpected proc %1$ju!=%2$ju"),
> (uintmax_t)vm->pid, (uintmax_t)kev.ident);
> goto cleanup;
> }
>
> ...
Thanks, I re-wrote it this way, it definitely looks better and less confusing.
> b) Handle the critical section in a separate function:
>
> if (kev.filter == EVFILT_PROC && (kev.fflags & NOTE_EXIT) != 0) {
> virObjectLock(vm);
> virBhyveProcessHandleExitEvent(vm, kev);
> virObjectUnlock(vm);
> }
>
> ...
>
> Or just keep it as is if you're fine with it. It just stuck out to me
> when I noticed it. So either way
>
> Reviewed-by: Martin Kletzander <[email protected]>
