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
