Send inn-workers mailing list submissions to
[email protected]
To subscribe or unsubscribe via the World Wide Web, visit
https://lists.isc.org/mailman/listinfo/inn-workers
or, via email, send a message with subject or body 'help' to
[email protected]
You can reach the person managing the list at
[email protected]
When replying, please edit your Subject line so it is more specific
than "Re: Contents of inn-workers digest..."
Today's Topics:
1. Re: Adding new gcc warnings (Julien ?LIE)
2. Re: Adding new gcc warnings (Julien ?LIE)
3. Re: Adding new gcc warnings (Julien ?LIE)
----------------------------------------------------------------------
Message: 1
Date: Mon, 20 May 2013 21:33:13 +0200
From: Julien ?LIE <[email protected]>
To: [email protected]
Subject: Re: Adding new gcc warnings
Message-ID: <[email protected]>
Content-Type: text/plain; charset=UTF-8
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)
------------------------------
Message: 2
Date: Mon, 20 May 2013 21:33:49 +0200
From: Julien ?LIE <[email protected]>
To: [email protected]
Subject: Re: Adding new gcc warnings
Message-ID: <[email protected]>
Content-Type: text/plain; charset=UTF-8; format=flowed
Hi Russ,
> Based on this discussion, I read through the other options, and am now
> trying:
>
> # A set of flags for warnings. Add -O because gcc won't find some warnings
> # without optimization turned on. Desirable warnings that can't be turned
> # on due to other problems:
> #
> # -Wconversion http://bugs.debian.org/488884 (htons warnings)
> #
> # Last checked against gcc 4.7.2 (2013-04-22). -D_FORTIFY_SOURCE=2 enables
> # warn_unused_result attribute markings on glibc functions on Linux, which
> # catches a few more issues.
> WARNINGS = -g -O -D_FORTIFY_SOURCE=2 -Wall -Wextra -Wendif-labels \
> -Wformat=2 -Winit-self -Wswitch-enum -Wuninitialized -Wfloat-equal \
> -Wdeclaration-after-statement -Wshadow -Wpointer-arith \
> -Wbad-function-cast -Wcast-align -Wwrite-strings \
> -Wjump-misses-init -Wlogical-op -Wstrict-prototypes \
> -Wold-style-definition -Wmissing-prototypes -Wnormalized=nfc \
> -Wpacked -Wredundant-decls -Wnested-externs -Winline -Wvla -Werror
Just a few remarks:
-Werror can be placed before -Wall because it appears before -Wall in
the GCC manual.
-Wendif-labels is not necessary because it is the default behaviour.
-Wuninitialized is not necessary because it is implied by -Wall.
-Wdeclaration-after-statement can sometimes be useful for clarity (for
instance declaring a variable only in the loop it is used). I do not
know whether this warning should really be enforced.
-Wlogical-op led to unclear code when fixed (according to the other mail
you wrote about INN; unless I misunderstood your meaning?)
-Wredundant-decls is not used in INN because of noise generated by
system headers.
-Wmissing-declarations and -Wsync-nand could be added.
Couldn't also the combination "-Wconversion -Wno-sign-conversion" be added?
Couldn't -Wstack-protector along with -fstack-protector be useful?
--
Julien ?LIE
? ? C'est joli cette avenue le long de la mer? ?a s'appelle
comment ?
? La promenade des Bretons. ? (Ast?rix)
------------------------------
Message: 3
Date: Mon, 20 May 2013 21:43:38 +0200
From: Julien ?LIE <[email protected]>
To: [email protected]
Subject: Re: Adding new gcc warnings
Message-ID: <[email protected]>
Content-Type: text/plain; charset=UTF-8; format=flowed
Responding to myself:
> -Wlogical-op led to unclear code when fixed (according to the other mail
> you wrote about INN; unless I misunderstood your meaning?)
I read too fast. You clearly said "it's worth fixing code that triggers
-Wlogical-op because it's often unclear", so this warning should be used.
--
Julien ?LIE
? ? C'est joli cette avenue le long de la mer? ?a s'appelle
comment ?
? La promenade des Bretons. ? (Ast?rix)
------------------------------
_______________________________________________
inn-workers mailing list
[email protected]
https://lists.isc.org/mailman/listinfo/inn-workers
End of inn-workers Digest, Vol 51, Issue 1
******************************************
