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

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

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.