[ccan] Should we start using -Wextra in CCAN?

[Tim Post]

Hi,

On Fri, 2009-05-15 at 03:51 -0400, Joseph Adams wrote:
> Building with -Wall -Wextra (a.k.a. -Wall -W) adds extra warnings like
> "Comparison between signed and unsigned" and "Unused variable foo"
> that are not present in -Wall by itself.  This can be a major nuisance
> when trying to use a CCAN module in a project that uses -Wextra.

-Wextra is really annoying, especially when you _know_ those comparisons
are only reached when two variables are signed the same. However, some
of the stuff that -Wextra turns on, such as -Wempty-body can be really
useful.

i.e. :

 if (foo == NULL);
    foo = strdup(bar);

You'll spend an hour looking for that leak with valgrind, just due to a
simple typo. -Wempty-body would catch that.

> Would it be a good idea to start using -Wextra in CCAN?  I went ahead
> and did the grunt work for this by turning on -Wextra wherever -Wall
> was found and spewing casts and voids all over the CCAN repository
> code.

Its not better to just use (some) of the more useful things that -Wextra
turns on? Casting something just for the purpose of shutting a compiler
up seems a little icky.

For instance, something like this:

int ret;
unsigned int i;

ret = foo(); /* foo is known to return a signed errno on failure */

if (ret < 0)
  return 1;

for (i = 0; i < ret; i ++)
   .....

That is one of the reasons that I never turn on -Wextra. You either have
to make i signed, or cast ret as unsigned, and it just seems kind of
silly to do either. Plus, it makes sensible optimizations more of a
chore.

So (at least) -Wempty-body gets my vote, but please ... not -Wextra :)

Cheers,
--Tim