On Wed, 3 Aug 2022 at 12:44, Daniel P. Berrangé <berrange@xxxxxxxxxx> wrote:
> Inconsistent return value checking is designed-in behaviour for
> QEMU's current Error handling coding pattern with error_abort/fatal.
Yes; I habitually mark as false-positive Coverity reports about
missing error checks where it has not noticed that the error
handling is done via the errp pointer.
> If we wanted to make it consistent we would need to require that
> all methods with 'Error **errp' are tagged 'attribute(unused)'
> and then provide
>
> # define ignore_value(x) \
> (__extension__ ({ __typeof__ (x) __x = (x); (void) __x; }))
>
> Such that if you want to use error_abort/error_fatal, you
> need to explicitly discard the return value eg
>
> ignore_value(some_method(&error_abort));
>
> If I was starting QEMU fresh, I think like the attribute(unused)
> anntation and explicit discard, but to retrofit it now would
> require updated about 3000 current callers which pass &error_abort
> and &error_fatal.
>
> On the flipside I am willing to bet that doing this work would
> identify existing bugs where we don't pass error_abort/fatal
> and also fail to check for failure. So there would be potential
> payback for the vast churn
Yes, if somebody wanted to take on the task of making this stuff
consistent I have no objection there. (I would be inclined to
suggest that this might involve a design where we consistently
report and check for the error in exactly one way, not in two
parallel ways.)
-- PMM
