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 (Russ Allbery)
2. Re: Adding new gcc warnings (Russ Allbery)
----------------------------------------------------------------------
Message: 1
Date: Mon, 22 Apr 2013 14:42:56 -0700
From: Russ Allbery <[email protected]>
To: [email protected]
Subject: Re: Adding new gcc warnings
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8
Julien ?LIE <[email protected]> writes:
> As gcc 4.4.x is currently used for the daily generation of snapshots,
> maybe we could add new warnings for "make warnings" builds.
The system will get upgraded to wheezy as soon as I have some time,
although based on current trends that might still be a few more months.
> I suggest the following flags:
> -Wformat-y2k
I recommend -Wformat=2 instead. It includes that and a bunch of other
useful stuff.
> -Winit-self
I always left this off because if I were to ever do that, it would be
intentional; it's a hard mistake to make by accident. But I don't recall
ever using it, so I have no objections.
> -Wold-style-definition
Oh, huh, that's not part of the default set?
> -Wmissing-declarations
I use -Wmissing-prototypes. I'm not sure what the difference is. The
documentation is not particularly illuminating.
> -Wnormalized=nfc
> -Wpacked
> -Winline
> -Wsync-nand
> -Wvla
These all seem fine, although probably won't trigger with our code (except
for the -Wpacked point that you note below).
> Should -Wswitch-default be added? It implies that all switch statements
> are required to have a default case, even though all possible cases are
> correctly handled.
I do tend to use this, but it can be quite annoying if you have large
enums and you're not currently handling every case. Basically, the more
you intentionally use default, the more annoying this is.
> Should -Wfloat-equal be added? Errors like these ones appear, and
> I wonder whether a fix is needed.
> In innd/status.c, size is a float:
> if (!size) size = 1; /* avoid divide by zero here too */
Since this is avoiding division by zero, checking against epsilon probably
isn't really needed, but it also probably wouldn't be hard. IIRC, this is
only a float due to possible calculation size problems with a long.
> In innfeed/host.c:
> if (h->params->dynBacklogFilter != oldBacklogFilter)
This should really be checking against an epsilon.
> -Wunreachable-code, -Wstrict-overflow=2, -Wtraditional-conversion and
> -Wlogical-op give false positives, so I believe adding them is not wise.
Here's what I currently use, FYI:
WARNINGS = -g -O -D_FORTIFY_SOURCE=2 -Wall -Wextra -Wendif-labels \
-Wformat=2 -Winit-self -Wswitch-enum -Wdeclaration-after-statement \
-Wshadow -Wpointer-arith -Wbad-function-cast -Wcast-align \
-Wwrite-strings -Wjump-misses-init -Wlogical-op \
-Wstrict-prototypes -Wmissing-prototypes -Wredundant-decls \
-Wnested-externs -Werror
I do think it's worth fixing code that triggers -Wlogical-op because it's
often unclear.
-Wtraditional-conversion warns about problems when upgrading from K&R to
ANSI C and isn't something we really need to worry about.
> 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.
> -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.
--
Russ Allbery ([email protected]) <http://www.eyrie.org/~eagle/>
Please send questions to the list rather than mailing me directly.
<http://www.eyrie.org/~eagle/faqs/questions.html> explains why.
------------------------------
Message: 2
Date: Mon, 22 Apr 2013 14:59:43 -0700
From: Russ Allbery <[email protected]>
To: [email protected]
Subject: Re: Adding new gcc warnings
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8
Russ Allbery <[email protected]> writes:
> Julien ?LIE <[email protected]> writes:
>> -Winit-self
> I always left this off because if I were to ever do that, it would be
> intentional; it's a hard mistake to make by accident. But I don't recall
> ever using it, so I have no objections.
And, as it turns out, I started using it for my other projects somewhere
along the line. :)
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
with other packages. (The order of flags is intended to match the order
in which they're presented in the GCC manual, which appears to be
semi-random.)
--
Russ Allbery ([email protected]) <http://www.eyrie.org/~eagle/>
Please send questions to the list rather than mailing me directly.
<http://www.eyrie.org/~eagle/faqs/questions.html> explains why.
------------------------------
_______________________________________________
inn-workers mailing list
[email protected]
https://lists.isc.org/mailman/listinfo/inn-workers
End of inn-workers Digest, Vol 50, Issue 6
******************************************
