Re: [PATCH 12/21] qga: Fix latent guest-get-fsinfo error handling bug

[Markus Armbruster]

Vladimir Sementsov-Ogievskiy <address@hidden> writes:

> 30.11.2019 22:42, Markus Armbruster wrote:
>> build_guest_fsinfo_for_virtual_device() crashes when
>> build_guest_fsinfo_for_device() fails and its @errp argument is null.
>> Messed up in commit 46d4c5723e "qga: Add guest-get-fsinfo command".
>> 
>> The bug can't bite as no caller actually passes null.  Fix it anyway.
>> 
>> Cc: Michael Roth <address@hidden>
>> Signed-off-by: Markus Armbruster <address@hidden>
>
>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>
> Actually, all such bugs should be fixed by my auto-generated series..

I see.  I didn't consider that.

One advantage of my manual fixing is a clearer record of the flaws in
git.  It should also keep your work simpler, which is always a good idea
for massive, mechanical patches.

> And, if fixing by hand, it may be better to teach this function to return
> int, than propagation is not needed.

I went for the minimal fix.

I believe returning something useful is better.  It also matches the
GError conventions.  Deviating from them in this point was a mistake.

Touching up functions to return something useful by hand is a lot of
work, though: functions have many returns, and we have many functions in
need of this touch-up.  Some clever Coccinellery might be able to pull
it off.  I haven't tried.

However, your clever "auto propagation" Coccinellery makes such a change
less compelling, because it hides the propagation.

Thanks!