Building Drizzle cleanly with all the warnings
I’ve been spending the last several weeks hacking on Drizzle, which is based on recent code from MySQL 6.0 and which Brian announced earlier today.
I’ve been having tons of fun, and have cleaned up lots of stuff which had bugged me for a while. I’m currently trying to end the reliance of the client library on mysys and mystrings… but that’s a story for later. One of the things I’m happiest about so far is that we are currently building with:
-W -Wall -Wextra -std=gnu++98 -pedantic -Wundef -Wredundant-decls -Wno-long-long -Wno-strict-aliasing -Werror
for C++ and:
-W -Wall -Wextra -std=gnu99 -pedantic -Wundef -Wstrict-prototypes -Wmissing-prototypes -Wmissing-declarations -Wredundant-decls -Wno-strict-aliasing -Werror
for plain C. In and of itself turning on a crapload of warnings did not solve any major problems… but going through and fixing them did point out some shady things, some places where code was unclean, etc. I really love though that we are have -Werror on now, which turns warnings in to errors, so that anyone writing crappy code can’t just say “oh, that one doesn’t matter” - they actually have to fix it.
You might notice that we have -Wno-strict-aliasing, to turn of strict aliasing warnings. I’d love to fix all of these, but there are so many places where we cast things to (char *) (why not void * at least?) to pass them around it’s not even funny. (I’d be happy to work with anyone who wants to help clean those up, btw)
Why bother turning on warnings, even if many of them are just annoying things like “unused parameters” or the like? Well, a couple of things. Once it’s clean, then it’s more checking at compile time that you’ve possibly done something silly. Second, if you do have tons of unused parameters, and you’ve added tons of __attribute__((__unused__)) in your code (which are thankfully very ugly) - you might notice that perhaps something isn’t designed so well and should be refactored.
They are warnings, though, so not all of them are dangerous, or bad - or even correct. But now that’s it’s “clean” - we’ll keep it that way. Yippee.
Leave a Reply
You must be logged in to post a comment.