Re: Add macros for diagnostic control, use them in locale/weightwc.h

[Joseph Myers <joseph at codesourcery dot com>]

On Mon, 24 Nov 2014, Roland McGrath wrote:

> If GCC's limitations make it infeasible to narrow the suppression
> scope very much, and/or make it impossible to use a surrounding macro
> to do so like my idea, then those ideas are pretty much out the
> window.  The pragma syntax is a bit verbose and clunky, so some macros
> to make things more concise are still probably worthwhile.

The surrounding macro doesn't provide the interval of lines with the 
pragma enabled, which is needed for it to work.

(And in some cases, a pragma for the whole file may make sense - if a test 
is deliberately testing deprecated interfaces or _FORTIFY_SOURCE runtime 
handling, disabling corresponding warnings for the whole file seems 
appropriate.)

> The bottom line is that I want it to be tractable and not gratuitously
> cumbersome to suppress particular warnings, but I want it to be
> difficult to do so indiscriminately or accidentally and easiest to
> maintain and change the code such that we won't inadvertently have
> warnings suppressed for new code.  I'm far less concerned about
> warnings in test code.  So, if outside of test code we in fact have a
> very small number of warnings in need of suppression, perhaps I've
> made a mountain of a molehill.

We have the weightwc.h warnings for which Paul has found another fix.  We 
have the nscd/connections.c "ignoring return value of 'setuid', declared 
with attribute warn_unused_result [-Wunused-result]" (and similar for 
setgid), in a context where another id-setting call has already failed, 
and while you could assign to a dummy variable it's not clear that has any 
advantage over explicitly suppressing the warnings.  We have, for some 
architecture/compiler combinations, "res_send.c:795:41: warning: 'resplen' 
may be used uninitialized in this function [-Wmaybe-uninitialized]" (a 
case where the relevant code only gets executed if the variable is 
initialized - the first time round a loop will execute a different if 
branch and cause it to be initialized).  And that's it for warnings (on 
x86_64 / ARM / PowerPC, in my testing) for which suppression seems a 
sensible first approach.

Maybe there would be more arising from conversion of -Wno- options in 
makefiles to use of macros / pragmas - but if so, high standards simply 
imply it's hard to move from suppression for a whole file to more 
selective suppression.

> Regardless of the mechanics we settle on, I want to say some things
> about review standards for new warning suppressions.  The bar to
> adding a warning suppression should be quite high.  It should be

I think we should be perfectly willing to file a bug in Bugzilla and then 
add a suppression referencing the bug, if there's something tricky about 
determining if there's a real glibc bug there or a better way to address 
the warning - that means the build stays working on more platforms (in the 
presence of -Werror).  I see no reason why we should assume a possible 
problem in existing code shown up by a warning (from new GCC or on another 
architecture) is more serious than other bugs.  -Werror is first about 
stopping new issues getting in accidentally, rather than forcing certain 
existing issues (those that cause warnings) to be higher priority than 
other existing issues.

(For example, on PowerPC you get

../sysdeps/ieee754/ldbl-128/e_lgammal_r.c:75:1: warning: floating constant 
exceeds range of 'long double' [-Woverflow]
../sysdeps/ieee754/ldbl-128/e_lgammal_r.c:78:1: warning: floating constant 
exceeds range of 'long double' [-Woverflow]

and while the right long-term fix is certainly *not* a warning 
suppression, it would seem reasonable as an interim fix for build breakage 
for an indefinite amount of time, with the comment referring to bug 
16347.)

-- 
Joseph S. Myers
joseph@codesourcery.com