Adding new gcc warnings
Julien ÉLIE
julien at trigofacile.com
Mon May 20 19:33:13 UTC 2013
Hi Russ,
>> I suggest the following flags:
>> -Wformat-y2k
>
> I recommend -Wformat=2 instead. It includes that and a bunch of other
> useful stuff.
Noted. The reason why I did not suggest -Wformat=2 is because of
a few "format-nonliteral" errors that are currently generated.
"warn if the format string is not a string literal and so cannot be
checked, unless the format function takes its format arguments as a
va_list."
Fixing these errors would be needed before enforcing -Wformat=2.
I will try to have a look at that.
>> -Wold-style-definition
>
> Oh, huh, that's not part of the default set?
-Wold-style-declaration is enabled by -Wextra.
However, -Wold-style-definition is not in the default set.
>> -Wmissing-declarations
>
> I use -Wmissing-prototypes. I'm not sure what the difference is. The
> documentation is not particularly illuminating.
According to:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50134
"it's a style warning: a global function should be
declared in a header, so warn for
int
f (void)
{
return 0;
}
if there was no previous declaration for f. (For C++, the definition and
any previous declaration will always provide a prototype.)
As Ian said in <http://gcc.gnu.org/ml/gcc/2011-08/msg00366.html>, for C++
the two options reduce to the same thing because no non-prototype
declarations or definitions exist. For C,
int f();
int f(void) { return 0; }
gets a warning with -Wmissing-prototypes but not -Wmissing-declarations,
because "int f();" is a non-prototype declaration in C."
>> In innfeed/host.c:
>> if (h->params->dynBacklogFilter != oldBacklogFilter)
>
> This should really be checking against an epsilon.
Should we then define EPSILON = 0.001 and change for instance the above check
to (abs(h->params->dynBacklogFilter - oldBacklogFilter) > EPSILON)?
or do you recommend something else to fix these issues?
>> Incidentally, a warning is triggered by -Wpacked in include/inn/dbz.h:
>>
>> typedef struct {
>> char hash[DBZ_INTERNAL_HASH_SIZE];
>> } PACKED erec;
>>
>> Can the PACKED attribute be removed? I do not understand its usefulness
>> for a struct of only one variable.
>
> I left the PACKED on there because I was afraid that removing it might
> change the layout of the data structure on disk on some platform, thus
> invalidating old history files. I think it's fairly unlikely that this is
> a problem, though.
So it is perhaps wiser to keep the PACKED attribute here. I will add
a comment above this code to mention that.
>> -Wold-style-definition returns an issue in innd/icd.c:
>>
>> char *
>> ICDreadactive(endp)
>> char **endp;
>> {
>>
>> Can it be replaced with the following definition?
>>
>> char *
>> ICDreadactive(char **endp)
>> {
>
> Yes, absolutely.
OK, thanks.
--
Julien ÉLIE
« – C'est joli cette avenue le long de la mer… Ça s'appelle
comment ?
– La promenade des Bretons. » (Astérix)
More information about the inn-workers
mailing list