Adding new gcc warnings

[Russ Allbery]

More than four years later, I finally took a look at these changes to the
INN warning flags.  :)  I think most of these changes were already made in
INN, but I also updated rra-c-util in places.

Julien ÉLIE <julien at trigofacile.com> writes:

> -Wendif-labels is not necessary because it is the default behaviour.

> -Wuninitialized is not necessary because it is implied by -Wall.

Also now dropped in rra-c-util.

> -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.

Yeah, this has now been adopted as standard C.  The risk is that INN would
fail to compile on older C compilers that follow the previous standard
(although I think GCC has always supported this).  I still avoid this
coding style myself, but I'm happy for INN to adopt declaration at use.

Note that -Wdeclaration-after-statement only applies at block scope, so a
variable declared at the top of a loop should be fine, since that's the
first statement inside a new block.

> -Wlogical-op led to unclear code when fixed (according to the other mail
> you wrote about INN; unless I misunderstood your meaning?)

It's a little weird, but I think it's worth it for the warnings about
using boolean operators as truth operators, which occasionally catches
some bugs.  It's odd code, but only in one place:

--- innd/rc.c   (revision 10202)
+++ innd/rc.c   (working copy)
@@ -454,8 +454,10 @@
     /* Get the connection. */
     size = sizeof remote;
     if ((fd = accept(cp->fd, (struct sockaddr *)&remote, &size)) < 0) {
-       if (errno != EWOULDBLOCK && errno != EAGAIN)
-           syslog(L_ERROR, "%s cant accept RCreader %m", LogName);
+       /* Avoid -Wlogical-op warnings if EWOULDBLOCK == EAGAIN. */
+       if (errno != EWOULDBLOCK)
+           if (errno != EAGAIN)
+               syslog(L_ERROR, "%s cant accept RCreader %m", LogName);
        return;
     }

> -Wredundant-decls is not used in INN because of noise generated by system
> headers.

I've now fixed this.  The noise from the system headers were really our
fault due to some old legacy compatibility code that's fairly pointless
now.

> -Wmissing-declarations and -Wsync-nand could be added.

Added -Wmissing-declarations to rra-c-util.  I haven't checked to see how
many warnings would be generated in INN.

-Wsync-nand is specifically to warn about a change in semantics of a GCC
built-in that we never used.  I'm in general in favor of enabling every
warning possible that makes sense for our coding style, but this cannot
possibly trigger in our code base and I can't imagine a situation when it
would, so doesn't seem worth the effort of probing for it.

> Couldn't also the combination "-Wconversion -Wno-sign-conversion" be added?

> Couldn't -Wstack-protector along with -fstack-protector be useful?

Added these to rra-c-util.  -Wconversion -Wno-sign-conversion will
probably require adding some cases; I had to make changes to both
rra-c-util and C TAP Harness for those.

-- 
Russ Allbery (eagle at eyrie.org)              <http://www.eyrie.org/~eagle/>