4 ms·

Nice summary. If stability/security is important in your codebase. It's of course worth avoiding undefined behavior, and compiling with warnings as errors (-We

by new299 10y ago

Nice summary. If stability/security is important in your codebase. It's of course worth avoiding undefined behavior, and compiling with warnings as errors (-Werror in gcc). However it's also really worth running your test suite through Valgrind [1] too sometimes. This instruments your code to check for use for use of initialized variables/other non-deterministic behavior at runtime. I used to run this under continuous integration, and flag builds with issues. [1] http://valgrind.org/ http://valgrind.org/

DSMan195276 10y ago
I agree that all warnings should be generally be turned on and snuffed out (Though there are a few innocuous warnings that I turn off sometimes), however I wouldn't recommend the usage of -Werror to force that unless it's only enabled for dev builds. Warnings are generally impossible to fully predict: different compilers can give different warnings - from warnings enabled by default, warnings changed between versions, and warnings that differ between compilers. And of course, there's no wrong way for the compiler to do it because what to warn about is subjective in the first place. That can become really annoying for an end user looking to just compile and run your software, not debug why it fails to build because their setup outputs a warning somewhere that may not even be an error. And turning off -Werror tends to then require delving into Makefiles (or whatever other build system they chose to use), and at that point most people are probably just not going to bother and call it broken. Again, I'm not saying you shouldn't attempt to snuff out and find every warning that you can, but you shouldn't have your default build rely on compiling with no warnings. There's just too many variables uncounted for to expect your software to compile without warnings in every possible configuration. I also agree 100% with running test suites through valgrind - I do that as well and it's a very good way to guard against memory leaks being introduced (Of course, good structure and design techniques are always the best defense, but there's nothing wrong with a good check that things are doing good).
- new299 10y ago
  Right, I think that's reasonable. Maybe just use it for debug or builds of test suites/CI builds.
- ryandrake 10y ago
  Another reason to avoid -Werror is if you pull in third party code that you share your build options with. Even large stable open source projects regularly spew hundreds of warnings when you aggressively turn them on. I'll generally build my own code with all warnings, but use as close as I can to the package maintainer's build settings for their code. Even then, you can get warnings because the third party developer doesn't use your exact compiler.
lomnakkus 10y ago
AFAICT valgrind is mostly obsolete; use ASAN+UBSAN+... instead.
- pjmlp 10y ago
  When gcc or clang are an option.
- planteen 10y ago
  Is the thread sanitizer caught up to where valgrind (helgrind) was for detecting data races? I know a while back it wasn't but things may have changed.
pjmlp 10y ago
The problem with Valgrind is that not everyone is working on one of these platforms http://valgrind.org/info/platforms.html http://valgrind.org/info/platforms.html
- abecedarius 10y ago
  It's also not enough. A medium-large program of mine that passed Valgrind had been working for years when it started blowing up on newer compiler releases. Maybe the newer static undefined-behavior analyses would have made that less of an awful pain, if they'd been available then.
buserror 10y ago
-Werror used to be the way to go, until compiler writers decided to add warnings for idiotic reasons. Like 'confusing indentation' or 'unused static const variables'. Seriously, these two are completely idiotic; why would the compiler decide that the fact I don't have a blank line 'might' be confusing? on whose criteria? Same with static const variables in C files. Lots of code is often #ifdef in/out, and lots of people would rather have a static const int than a #define!
- mattkrause 10y ago
  The "confusing indentation" warning can be useful. For example, it would have flagged if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0) goto fail; goto fail; Does any compiler try to indicate the severity of each warning. For example, using the value from an uninitialized variable is labeled as somehow "worse" than code-quality warnings like indentation or bare assignments inside conditionals?

by new299 10y ago

DSMan195276 10y ago

I agree that all warnings should be generally be turned on and snuffed out (Though there are a few innocuous warnings that I turn off sometimes), however I wouldn't recommend the usage of -Werror to force that unless it's only enabled for dev builds. Warnings are generally impossible to fully predict: different compilers can give different warnings - from warnings enabled by default, warnings changed between versions, and warnings that differ between compilers. And of course, there's no wrong way for the compiler to do it because what to warn about is subjective in the first place. That can become really annoying for an end user looking to just compile and run your software, not debug why it fails to build because their setup outputs a warning somewhere that may not even be an error. And turning off -Werror tends to then require delving into Makefiles (or whatever other build system they chose to use), and at that point most people are probably just not going to bother and call it broken. Again, I'm not saying you shouldn't attempt to snuff out and find every warning that you can, but you shouldn't have your default build rely on compiling with no warnings. There's just too many variables uncounted for to expect your software to compile without warnings in every possible configuration. I also agree 100% with running test suites through valgrind - I do that as well and it's a very good way to guard against memory leaks being introduced (Of course, good structure and design techniques are always the best defense, but there's nothing wrong with a good check that things are doing good).

new299 10y ago
Right, I think that's reasonable. Maybe just use it for debug or builds of test suites/CI builds.
ryandrake 10y ago
Another reason to avoid -Werror is if you pull in third party code that you share your build options with. Even large stable open source projects regularly spew hundreds of warnings when you aggressively turn them on. I'll generally build my own code with all warnings, but use as close as I can to the package maintainer's build settings for their code. Even then, you can get warnings because the third party developer doesn't use your exact compiler.

lomnakkus 10y ago

AFAICT valgrind is mostly obsolete; use ASAN+UBSAN+... instead.

pjmlp 10y ago
When gcc or clang are an option.
planteen 10y ago
Is the thread sanitizer caught up to where valgrind (helgrind) was for detecting data races? I know a while back it wasn't but things may have changed.

pjmlp 10y ago

The problem with Valgrind is that not everyone is working on one of these platforms http://valgrind.org/info/platforms.html http://valgrind.org/info/platforms.html

abecedarius 10y ago
It's also not enough. A medium-large program of mine that passed Valgrind had been working for years when it started blowing up on newer compiler releases. Maybe the newer static undefined-behavior analyses would have made that less of an awful pain, if they'd been available then.

buserror 10y ago

-Werror used to be the way to go, until compiler writers decided to add warnings for idiotic reasons. Like 'confusing indentation' or 'unused static const variables'. Seriously, these two are completely idiotic; why would the compiler decide that the fact I don't have a blank line 'might' be confusing? on whose criteria? Same with static const variables in C files. Lots of code is often #ifdef in/out, and lots of people would rather have a static const int than a #define!

mattkrause 10y ago
The "confusing indentation" warning can be useful. For example, it would have flagged if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0) goto fail; goto fail; Does any compiler try to indicate the severity of each warning. For example, using the value from an uninitialized variable is labeled as somehow "worse" than code-quality warnings like indentation or bare assignments inside conditionals?