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

[Philippe Mathieu-Daudé]

On 10/11/23 12:20, 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);

Rather:

       g_assert_not_reached();

>    }
>
> 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.