Hi Collin,
> > For test cases this is more a judgment call, but I prefer doing either
> > the above or adjusting the warning flags, to ignoring warnings, as the
> > other warnings can be useful at time.
>
> Yeah, I could see these warnings making it hard to see ones that
> actually matter. Lets see what Bruno thinks.
Different warning policies need to apply in these fours sets code:
1) Code that originates in the package that uses gnulib.
Example: coreutils/src/*
2) Code from public header files in gnulib/lib/
Example: lib/vasnprintf.h because module 'vasnprintf' designates this file
as its public header file.
3) All other code from gnulib/lib/
Example: lib/argp-namefrob.h which is not a public header file, lib/*.c.
4) All code in gnulib/tests/
Note that different warning policies may contradict each other. For example,
some people want to see a warning for
int *table = malloc (n * sizeof (int));
because it has an implicit conversion / "lacks a cast". While other people
want to see a warning for
int *table = (int *) malloc (n * sizeof (int));
because it has a cast and "casts are dubious". It is impossible to satisfy
both of these policies at the same time.
Back to the four sets of code:
1) This warning policy is the responsibility of that package's maintainer,
obviously.
2) These header files are used in compilation units of the package, with
CFLAGS or AM_CFLAGS set by the package's maintainer for that package.
Therefore in these files we need to avoid even -Wundef, -Wvla, and
other kinds of warnings that we wouldn't enable in our code.
3) The rest of the lib/ code is under our responsibility, not the
responsibility of a package's maintainer. We try to avoid warnings
from "reasonable" warning options. More details in the HACKING file.
4) The unit tests are also in our responsibility, not the responsibility
of a package's maintainer. Here, the primary concern is that is must
be *easy* to contribute new unit tests. -Wmissing-variable-declarations
warnings _could_ — as Paul wrote — be avoided by adding an 'extern'
declaration for each global variable. But this is extra effort that
would hinder the addition of new unit tests.
> If we decide to follow the coding style you mentioned
No. It must be possible to contribute a unit test with a simple
global variable. Therefore -Wmissing-variable-declarations is not
adequate for unit tests.
Collin, if you want to find relevant findings in the unit tests, by
using gcc or clang warning options, do *not* use a coreutils build
for this purpose, but a gnulib testdir instead. (Because the latter
is not biased by coding style preferences of any package maintainer.)
Or if you really want to use a coreutils build, first update the
GL_CFLAG_GNULIB_WARNINGS definition in m4/gnulib-common.m4, so that
it eliminates useless kinds of warnings.
Bruno
