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 <mkletzan@redhat.com>