bug-gnulib
[Top][All Lists]
Advanced

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

Re: [PATCH] Avoid one "parameter unused" warnings.


From: Michal Privoznik
Subject: Re: [PATCH] Avoid one "parameter unused" warnings.
Date: Tue, 2 Jan 2018 07:21:37 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0

On 01/01/2018 07:41 PM, Paul Eggert wrote:
> Michal Privoznik wrote:
>> I'm failing to see why this would be a
>> false positive.
> 
> It's a false positive in the sense that there is no bug here. That is,
> the Gnulib patch would mean that we would be spending maintenance effort
> to complicate code merely to pacify a compiler, not to fix a real
> problem. Sometimes this is worth doing, if the compiler's warning is
> typically a valid one and is therefore useful to catch bugs. However,
> this particular case does not seem to be worth doing, as this particular
> compiler warning is too often a false alarm and the bugs it catches are
> not worth the maintenance effort.

I beg to disagree. For reference I'm pasting skeleton of the function:

  inline in
  stat_time_normalize(int result, struct stat *st)
  {
  #if $cond

  #endif
    return result;
  }

If $cond is false (i.e. I'm not building on SUN), the @st variable is
unused indeed. True, it's not a serious error nor bug. However, it's not
a false positive either - we told compiler to warn us in this case and
it did. If you guys don't want to 'fix' it, that's okay. I was more
focused on the false positivity. BTW: there are already instances of my
patch (true, from ancient history), e.g. 81eb84868d.

> 
>> Despite this function being inlined, compiler might
>> decide to not inline it and thus treat it as any other function in which
>> case the variable is unused indeed.
> 
> I don't see why inlining is relevant. Regardless of whether the compiler
> decides to inline at the machine-code level, the extra source-code
> clutter does not fix any real bug.
> 
> I attempted to reproduce the problem by running these commands on Fedora
> 27 x86-64:
> 
> git clone git://libvirt.org/libvirt.git
> cd libvirt
> ./autogen.sh
> make -j5
> 
> This worked fine for me, so I can't easily test a fix; however, perhaps
> the attached patch to libvirt will fix things for you.

Try running 'make syntax-check' which fails because of gnulib's
copyright_check. And when updating the submodule I've found this bug.

BTW: the patch of yours don't help really. Firstly, unused-parameter
sneaks back in (through -Wextra I assume). And secondly, in libvirt we
want to use Wunused-parameter and we do mark unused parameters
explicitly (see ATTRIBUTE_UNUSED macro).

Michal



reply via email to

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