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

[Daniel P . Berrangé]

On Fri, Nov 10, 2023 at 12:35:56PM +0100, BALATON Zoltan wrote:
> 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);

Casting to void is not sufficient, which is why I suggested the
ignore_value macro, which does enough to fool the compiler into
thinking the return value is used.

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 :|