Vigilant Software Blog
Defect Spotlight: Useless checks
Are you interested in code quality? This post is part of a series called Defect Spotlight. In it, we'll be bringing you instructive examples of real-world defects found by static analysis. Learn from the mistakes shown here!
One of the advantages of a statically-compiled language like C or C++ is that a good deal of information is available at compile time. While robust software can be written in more dynamic languages, it requires a certain level of attention to detail and a very thorough suite of dynamic tests.
This extra compile-time information is often a blessing for catching bugs early. First of all, your compiler catches a number of stupid mistakes before you even get a chance to run any tests, from misspelled names to using incorrect data types.
We added a couple checks recently for simple mistakes like checking an unsigned` value for being less than zero, which is clearly impossible. You'd never do that, right? Of course, experience has taught us that there is no mistake that's too simple to make. Even the best engineers make simple mistakes on a fairly regular basis.
1 2 3 4 5 6 7 8 9 10 | len = g_socket_receive_message (nl->priv->sock, NULL, &iv, 1, NULL, NULL, &flags, NULL, &error); if (len < 0) { g_warning ("Error on netlink socket: %s", error->message); g_error_free (error); if (nl->priv->dump_networks) finish_dump (nl); return FALSE; } |
With the new scanner, Sentry found this perfectly innocent-looking
code in GLib. If
g_socket_receive_message returns a value less than zero, the
error-handling logic will clean up and return, right? Unforunately,
this code is unreachable because len was declared to be an unsigned
type (gsize), and can never have a value less than zero. The check
is useless because the condition is provably false at compile time.
(Sentry also checks for checks that are useless because they will
always be true — the else path could never be reached.)
When this code encountered a network I/O problem, it most likely would have caused undefined behavior in the program. The structure would not have been properly initialized and the code would've continued to execute as if it had been. Not only that, but it's the insidious kind of dead (unreachable) code that appears to be reachable. Debugging the problem could have taken hours trying to determine why the structure is (only sometimes) full of garbage. (We reported this bug to the GLib developers, and it was quickly fixed.)
It's a good reminder that automated checks for even very simple mistakes can produce valuable feedback. Taking advantage of the additional type information we have at compile time can save programmer hours and prevent entire classes of defects from impacting you in production.
Happy holidays from Vigilant
We've posted a bug-fix release of Sentry today, version 3.1.1. Our customers are encouraged to upgrade to this latest stable release at their convenience. As always, it's available from the download page now.
Happy holidays from Vigilant! Here's to a productive 2012!
Why ease of use is so important
Usability is central to our development philosophy, and something we've spent a lot of time perfecting. What's so important about it anyway? Obviously, good software strives to be easy to use, but why do we make such a big deal about it?
Our customers ramp up quicker
First of all, and probably most obviously, our customers get going much faster than with our competitors. A majority of them are able to install Sentry and analyze their code without asking us a single question. This is (sadly) unusual for tools in our space, and getting it right is something we take pride in.
Productivity tools are supposed to make you productive
What's the point of a tool that saves time in your development process if it's hard to use? Productivity tools should be easy to install, easy to integrate, and easy to monitor. We want our software to be a natural extension of your existing process.
Focus on what matters
When software is easy to use, it fades into the background. Good software gets out of your way and lets you do your job. We took this message to heart and made your code and your results the focus. We want our users to spend less time fiddling and configuring, more time getting stuff done.
Finally, ease of use isn't really about hand-holding. It's a conscious focus on removing redundancy and unnecessary steps, automating whatever can be automated, and, of course, eating our own dogfood.
How to actually fix a bug
A new bug report landed on your desk this morning. Maybe it's from an customer in the field, maybe QA found it, or maybe you found it and reported it yourself. (You are using issue tracking software, right?)
If you're experienced in this sort of thing, your first step is usually to attempt to reproduce the issue. How can you know if you've fixed the problem without first seeing it fail? Maybe you can't reproduce it and need more information, or maybe it's actually working as intended and the report is mistaken.
For this story, let's assume the bug report is a real defect and you are able to replicate it. What's your next step?
If you said "write a test", you get a cookie. Write a test and watch it fail. (You have a test suite, right?) Do not pass go; don't start debugging, don't start hacking around in the code, and definitely don't commit anything.
Write a test, watch it fail.
Once you have a failing test, you are free to start changing things. Diagnose and fix the bug, re-run your test suite. Not only should all the old tests pass (verifying your change didn't break anything), but the new test should pass (proving your "fix" actually fixes something).
This approach accomplishes a few things:
- It forces you to stop and actually reproduce the issue.
- It ensures that you've done a before and after test, and that your change actually fixes the issue.
- Finally, it prevents this issue from re-appearing in the future - you get a regression test for free by following this process.
It's frustrating to squash a bug, only to have it reappear later, right? This is how you make sure they stay dead. It's actually a form of test-driven development, an approach that is well-suited to bug fixes.
Sentry 3.1 released!
A new minor release of Sentry, our flagship static code analyzer, is available now!
What's New in Sentry 3.1.0
-
Improved C++11 support: The new C++11 standard has only recently been ratified, but Sentry already supports most of the new language features. Sentry 3.1 adds to this, improving our ability to parse and analyze C++11 programs.
-
Faster Capture: Sentry 3.1 speeds up the process of creating a new project by about 25% on all platforms. Additionally, capture times on Windows systems are cut by over 50% thanks to some efficiency improvements.
-
New Scanner: The new Shadowed Variable scanner looks for variables whose declarations overshadow a previously reachable variable (whether local, global, or at the class level). This plugin identifies all instances of this potentially confusing practice and helps make your code safer more readable.
-
Bug Fixes: While 3.1 includes new features and enhancements, this release also fixes several bugs found in earlier releases. All customers are encouraged to upgrade at their earliest convenience!
See the announcement for Sentry 3.0 to learn about the new features in this major release series.
How To Get Sentry:
Sentry 3.1 is available for Windows and Linux operating systems. Contact us to try Sentry free for 30 days! As always, Vigilant customers can get the latest release in the download area.