warning options

I’m a very vocal person when it comes to enabling warnings for compilation of C code. I’m also very vocal about using -Werror, but that’s a longer topic I won’t go into here. I guess I could give a talk about why it’s critically important for the quality of your code to use -Werror. No, I’m gonna talk about the warning flags I’m using in Swfdec.

An important thing to note about warnings is that they flag code that is perfectly valid. So enabling warnings restricts you. It complains even though you write perfectly valid C code. What it does however is help you avoid common errors and force you to write cleaner code. It’s especially good at making sure you write clean code from the start instead of “I’ll clean that up later” code.

-Wall
This is the default list of compiler warnings that even a lot of build scripts enable automatically. You should always enable it, as new warnings are only added to it when they clearly show bad code.

-Wextra
This is another set of compiler warnings. Most of them are very useful, some of them aren’t. I prefer to enable it and disable the not-so-useful ones (see below), as I profit from new warnings being added automatically.

-Wno-missing-field-initializers
This warning complains when you initialize sturcts or arrays only partially. It’s usual glib coding style to do just that:

GValue val = { 0, };

So I’ve disabled the warning. It’s important to know that partially initialized values do indeed initialize the rest of the value to 0, so the other values are by no means undefined.

-Wno-unused-parameter
This warns about parameters in functions that aren’t used in the function body. As vfuncs in classes and signal callbacks often have parameters that you don’t use, I prefer to disable this warning. (Note to self: file a bug against gcc so static functions that are used by reference don’t get flagged?)

-Wold-style-definition
This is a mostly useless warning to avoid really old code (and associated bugs). It disallows function definitiona like

void
foo (x)
  int x;
{
}

If you’ve ever seen this, you should consider yourself seriously oldschool.

-Wdeclaration-after-statement
This is a matter of taste. This warning enforces that all declarations are at the top of a function and you don’t do stuff like

for (int i = 0; i < x; i++)

It also ensures portability to old-school compilers.

-Wmissing-declarations
Warns if a non-static function is not declared previously. As non-static functions should always be exported in headers, the header will have the previous declaration. So this warning finds two important cases:

• You forgot to make the function static even though it should be. This happened very often to me before using this warning.
• You forgot to include the header that declares this function. This can cause very nasty side effects, when you change the function parameters but not the prototype. You won't get a warning if you forget the declaring header.

-Wmissing-prototypes is a less-strict warning of the same type and included in Swfdec's flags just because we can.

-Wredundant-decls
Warns if a declaration appears twice. This usually means you have either forgotten to put #ifdef MY_HEADER_H defines in your header or you moved functions to a different header and forgot to delete the old one.

-Wmissing-noreturn
Warns if functions that don't ever return (such as exit) are not flagged as such. Use G_GNUC_NORETURN to indicate those functions. This warning is mostly useless, but for me it managed to detect when I screwed up my ifs so that all code paths ended at a g_assert_not_reached() statement.

-Wshadow
A really really useful warning (that emits way too many warnings in gcc 3.x). It warns about code like this:

int x = 3;
/* lots of code */
if (foo) {
  int x = 5;
  /* lots more code */
  printf ("%d\n", x);
}

You might have wanted to print the x you declared first, not the second one. It's one of the bugs you'll have a very hard time finding. It's a bit annoying that functions such as index or y0 are part of the C standard, but you learn to avoid them.

-Wpointer-arith
Warns about adding/subtracting from void pointers. This usually works the same like char pointers, but it enforces that you manually cast it. The more important thing is that it catches you when a pointer was a void pointer even though you didn't intend it to, especially in macros. Stupid example:

MyExtendedGArray *array = some_value;
MyType *nth_element = array->data[n];

This is actually the reason why the g_array_index() takes the type of the array data.

-Wcast-align
This warning is useful for portable code. Some weird processors (read: ARM) need to have values aligned to the right addresses and just doing int i = *(int *) some_pointer; can cause this to fail. gcc will warn you about such code, but unfortunately only for the architecture you're compiling to. But still, if you include this warning, all the Nokia people interested in your code will tell you about it and then you can fix it.
Important side note: You can avoid this warning if you know that the alignment is correct by casting to an intermediate void pointer, like int i = *(int *) (void *) some_pointer;

-Wwrite-strings
Another very important warning. Strings like "Hello World" are constant in C and must not be modified. This warning makes all these strings be treated like const char * instead of just char *. This helps a lot in avoiding stupid things, but can require quite a bit of constness cleanup in ones own code if it's retrofit onto existing code. It's well worth doing the work however.

-Winline
Warns if a function declared as inline cannot be inlined. This warning should not ever triggered, because a function should never be declared as inline. Wether to inline or not is a decision of the compiler and not of some coder. So never use the inline attribute. Not even when you are writing boot sequences in an operating system.

-Wformat-nonliteral
Warns about stuff like printf (foo);. You should use printf ("%s", foo); instead, as the variable foo can contain format strings such as %s. The printf argument should always be a constant string.

-Wformat-security
Warns about usages of printf() and scanf() that can cause security issues. You absolutely do want this warning.

-Wswitch-enum
Warns if you a switch statement does not have case statements for every value in an enumeration. This warning is invaluable for enumerations that you intend to add values to later on, such as cairo_format_t, because adding the new value will cause warnings in all switch statements using it instead of using the default case - which (in my code at least) is often an assert_not_reached. You can avoid this warning by casting the enumeration to an int: switch ((int) value)

-Wswitch-default
Warns whenever there's no default case in a switch statement. I tend to forget these quite often. If you don't need a default case, just do:

default:
  break;

-Winit-self
Wrns if you initialize a variable with itself, like MySomething *sth = MY_SOMETHING (sth); (instead of MySomething *sth = MY_SOMETHING (object);). This was one of my favorite errors before using this warning.

-Wmissing-include-dirs
Warns if a specified include irectory does not exist. This usually means you need to improve your autotools-fu, because you seriously messed up.

-Wundef
Warn if an #undef appears for something that wasn't ever defined. This not only detects typos, but is most useful for clean code: undefs are after the code, so I often forget deleting it when removing a macro.

-Waggregate-return
Warns if a struct or array is used as a return value. Either return a pointer instead or preallocate the value to be filled and pass it to your function.

-Wmissing-format-attribute
Warns if a function looks like it should take printf-style arguments but isn't flagged with G_GNUC_PRINTF.

-Wnested-externs
Detects if you have used extern inside a function. Workaround: Include the right header. Or use the extern before the function.

You should probably read the definitive guide to compiler warnings, maybe you'll find more useful warnings that I've overlooked. You'll likely have a lot of a-ha experiences while reading it.

If you now decided you want to use these warnings in your code, what's the best way to do it? The best way I know is to grab David's awesome m4 script and put it in your m4 scripts directory. After that just use AS_COMPILER_FLAGS like this in your configure.ac:

AS_COMPILER_FLAGS (WARNING_FLAGS, "-Wall -Wswitch-enum -Wswitch-default")

This will check each of the flags you put there and put all the ones that work into the WARNING_FLAGS variable and ignores the ones that don't work. So it'll work perfectly fine with older or non-gcc compilers. And if you ever get annoyed by a flag or want to add new ones. you just edit the list.