On Mon, 2020-05-04 at 14:19 +0200, Michal Privoznik wrote: > I'm in the middle of review too. But I find !data.ret a bit confusing. > The function is returning a boolean and not a pointer or an int. So how > about 'return data.ret != 0'? I know John was against using 'if (!int)' > and it kind of makes sense. > > Or even better, we can use our regular pattern and keep @ret, and do > something like: > > if (data.ret < 0) > goto cleanup; > > vshPrintExtra() > ret = true; > ... > return ret; Yeah I don't like it either, but if you look at the rest of the file there are multiple occurrences of the same pattern, which we should address as well. I will push the minimal fix for now, so that it can make it into 6.3.0; afterwards, someone can post a patch that removes the pattern from the entire file. -- Andrea Bolognani / Red Hat / Virtualization