qemu-arm
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 2/2] Add warn_unused_result attr to AUD_register_card


From: BALATON Zoltan
Subject: Re: [PATCH 2/2] Add warn_unused_result attr to AUD_register_card
Date: Fri, 10 Nov 2023 12:35:56 +0100 (CET)

On Fri, 10 Nov 2023, Daniel P. Berrangé wrote:
On Fri, Nov 10, 2023 at 12:44:56PM +0200, Manos Pitsidianakis wrote:
On Fri, 10 Nov 2023 12:21, Peter Maydell <peter.maydell@linaro.org> wrote:
This kind of thing is why Coverity's unused-result warning has a
lot of false positives. We shouldn't introduce extra code like
this to work around the fact that the tooling doesn't understand
our error-handling convention (i.e. error_fatal, and the way
that some functions report errors both via the return value and
also via the Error** argument).

I respect that :). But I personally believe that clinging to C's
inadequacies, instead of preventing bugs statically just because it adds
some lines of code, is misguided. Proper code should strive to make bugs
impossible in the first place. At least that is my perspective and I would
like there to be constructive discussions about different approaches in the
mailing list. Perhaps something good might come out of it!

Your approach to the problem:

 if (!AUD_register_card("OMAP EAC", &s->codec.card, &error_fatal)) {
   exit(1);
 }

is adding dead-code because the exit(1) will never be reachable. So while
it lets you squelch the unused result warning, it is verbose and misleading
to anyone who sees it.

Perhaps a more viable option is to pull in gnulib's ignore_value macro

 #define ignore_value(x) \
   (__extension__ ({ __typeof__ (x) __x = (x); (void) __x; }))

and then we would have just this:

ignore_value(AUD_register_card("OMAP EAC", &s->codec.card, &error_fatal));

I wonder if just casting to (void) without assigning to a value could avoid the warning? In that case you would not need a macro just

(void)AUD_register_card("OMAP EAC", &s->codec.card, &error_fatal);

Not sure it's clearer than a macro which states what it does but the definition is a bit cryptic while this cast is simpler but may also puzzle somebody not familiar with it.

Regards,
BALATON Zoltan

reply via email to

[Prev in Thread] Current Thread [Next in Thread]