Most of these recommendations are specifically for Kerberos development; other projects should make decisions appropriate to the project.
For new projects, "-Wall -Werror" is probably a useful starting point. The "-Wextra" option should be considered as well. Optimization should be enabled (normal when using autoconf-based configure scripts), as that enables dataflow analysis and can help detect use of uninitialized variables, unreachable code, etc.
Very recent versions of gcc (more recent than Apple is shipping now) have options to selectively turn many of the warnings into errors with "-Werror=...". This can be used to force a build to stop if some of these warnings are tripped. I would recommend using those options (when they become available) for certain selected warnings once they've been eliminated.
For earlier versions of gcc, we can create a wrapper script that uses regexp matching to look for certain warning strings as output from the compiler. (Assume English; watch out for varying quotation strings that may depend on, e.g., whether the terminal appears to support UTF-8 output.) Use of the compiler command-line options (or both?) is probably preferable; while they clutter up the build logs, they take effect independent of language and without regard to tweaks to the message texts. However, I don't know if we get quite the same fine level of control as we get looking at the messages. (E.g., if a warning option enables or disables two or three related warnings, the -Werror* option may make all or none into errors, while we may want to make only some into errors. On the other hand, we may need patterns to match against "argument" and "variable" and other minor variations in wording of a general warning.)
Start with writing the wrapper script, add only one or two message patterns to check for at a time. Fix the sources so the tree builds again. Add more message patterns. Lather, rinse, repeat. (Maybe do a quick check to see if our tree is already clean of some of the warnings, and go ahead and add those.) When "-Werror=foo" becomes available on all the platforms we care about, reevaluate use of the script and consider discarding it.
When enabling warnings or using attributes in gcc, test that each is actually accepted by gcc; various warnings and attributes are being added all the time, and ICC emulates GCC in some ways. For anything going into installed header files, though, explicit compiler, version, and language tests (_GNUC, __INTEL_COMPILER, __GNUC_MINOR, __GNUC_PATCHLEVEL_, __cplusplus, etc) are necessary, because they may be used with other compilers besides the one we build with. At http://www.ohse.de/uwe/articles/gcc-attributes.html
there's a good list of function attributes supported in gcc versions 2.0 through 3.4.0; I haven't seen a table covering later versions too.
We can probably assume any GCC implementation is 3.0 or later, can't we? But ICC defines _GNUC_ too. I'm not sure what to do about that.
There may be cases where system header files trigger some warnings from correct application code. Try to suppress or disable the warnings on a system by system basis, perhaps with configure "feature" tests. Or, teach the wrapper script how to detect system-header filenames, and ignore warnings there.
Apple's current gcc has an option to turn the implicit-function-declaration warning into an error. Use it.
Use the "deprecated" attribute on more platforms. But suppress the warnings – either globally, or create a way to silence the warnings for our own header files – during our own build. (Better: Disable with a finer-grained granularity than "the whole build". I.e., warn for deprecated libkrb5 functions when used in other, higher-level libraries or in applications, even in our own tree.) Keep the warnings for installed headers.
During development, use -ftrapv. It actually misses a lot of cases, but if it catches any overflow issues, it would be useful to fix the bugs. (Q: Do we want to use different compiler defaults for development and release branches?)
Add gcc's -fstack-protector or -fstack-protector-all options (the former only works on functions with automatic char arrays), and maybe -Wstack-protector. Define _FORTIFY_SOURCE=2 (or 1, if 2 doesn't work) on Linux (or any GNU libc system).
Warnings to consider making errors initially:
- char-subscripts
- uninitialized variables used
- printf/scanf/etc format errors
- implicit fn decl, missing prototypes
- fn defined without prototype (make static or add to header)
- old-style fn definition
- implicit int variable or fn-return type
- mixing pointer and integral type of different size
- compile-time division by zero (if not already an error)
- sequence-point violations
- shortening 64-bit value implicitly to 32-bit (use explicit cast)
- assignment in condition without extra parens (?? but fix as (X=Y)!=0 or separate assignment out?)
- parens needed to clarify possibly-confusing expression
- assignments to casts or conditionals (GNU extensions)
- use of trigraphs
- multicharacter constants
- unrecognized pragmas (should be in #ifdef for correct compiler) Examine later:
- ?? missing braces in variable init (except, PTHREAD_MUTEX_INIT may be broken on Solaris here)
- unused variables, functions, parameters (see below)
- discarded or casted-away qualifiers (may require very minor API changes, e.g., add/remove "const")
- confusing type conversions (-Wconversion, -Wno-sign-conversion)
- redundant decls
- -Wstrict-overflow
- -Wclobbered
- -Wold-style-declaration (e.g., storage class anywhere but first)
- tokens after #endif Depending on coding standards:
- c99-style "//" comments (unless we decide to start allowing them)
- declaration-after-statement
- nested comments
- ?? shadowed variables (BUT! prototype arg names matching libc/OS headers probably trigger this, e.g., "log" or "sin")
- -Wnested-externs - "extern" not at file scope
- variadic macros
- -Wswitch-default, -Wswitch-enum or -Wswitch
Many of these are already enabled as warnings in our build process.
Make string literals have type "const char[]". (-Wno-write-strings)
Use "nonnull" attribute liberally in function prototypes. BUT we do use NULL explicitly in a couple internal cases where we know it's safe, while application code should get the warning. Also, if we want warnings for applications, but still want to explicitly check for NULL ourselves, we need to suppress the attribute in the library build.
Use "noreturn", "const", "pure", "sentinel" attributes if we have any suitable functions.
No warnings for returning structures or unions.
Attach "warn_unused_result" attributes to any key functions where results might be ignored and cause problems.
Suppressing unused-parameter warnings: GCC has variable attributes. Lint has /ARGSUSED/. Some tools accept "x = x" as "using" the value and will shut up. Examine the various tools we decide to use, before deciding how to resolve these.
Eliminate unused variables; if they're used based on preprocessor conditionals, declare them that way too, maybe moving into inner blocks.
Do NOT warn for unreachable code – our wrapper macros and functions will generate this in a bunch of places. We can try to rework them (and convert to inline functions) to reduce the occurrences, but I doubt it'll be easy to eliminate them, especially depending how we want to approach pre- and post-conditions.
See if strict-aliasing warnings generate too many complaints. We probably should investigate them, but we may be doing too much legitimate casting, in cases the compiler can't fully analyze, to silence the warnings.
Other warning options can help us improve function attribute annotations which in turn can better improve the compiler's ability to optimize or to warn about problems, but they aren't in any way real problems, directly. Still, making them "errors" calls our attention to places where we could improve things. Don't make them errors at first (unless there are currently no warnings generated), but maybe enable the warnings.
- -Wmissing-format-attribute
- -Wmissing-noreturn (low priority)
(In future: See also function attributes: error, warning, hot, cold, visibility. Also consider __builtin_expect.)