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