[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: |
Daniel P . Berrangé |
Subject: |
Re: [PATCH 2/2] Add warn_unused_result attr to AUD_register_card |
Date: |
Fri, 10 Nov 2023 11:20:27 +0000 |
User-agent: |
Mutt/2.2.9 (2022-11-12) |
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));
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|