Always initialize your variables
The difference between the situations you are considering is that the case without initialization results in undefined behavior, while the case where you took the time to initialize creates a well defined and deterministic bug. I cannot stress how extremely different these two cases are enough.
Consider a hypothetical example which may have happened to a hypothetical employee on a hypothetical simulations program. This hypothetical team was hypothetically trying to make a deterministic simulation to demonstrate that the product they were hypothetically selling met needs.
Okay, I'll stop with the word injections. I think you get the point ;-)
In this simulation, there were hundreds of uninitialized variables. One developer ran valgrind on the simulation and noticed there were several "branch on uninitialized value" errors. "Hmm, that looks like that could cause non-determinism, making it hard to repeat test runs when we need it most." The developer went to management, but management was on a very tight schedule, and couldn't spare resources to track down this issue. "We end up initializing all of our variables before we use them. We have good coding practices."
A few months before the final delivery, when the simulation is in full churn mode, and the entire team is sprinting to finish all the things management promised on a budget that, like every project ever funded, was too small. Someone noticed that they couldn't test an essential feature because, for some reason, the deterministic sim wasn't behaving deterministically to debug.
The entire team may have been halted and spent the better part of 2 month combing the entire simulation codebase fixing uninitialized value errors instead of implementing and testing features. Needless to say, the employee skipped the "I told you so's" and went straight into helping other developers understand what uninitialized values are. Strangely enough, the coding standards were changed shortly after this incident, encouraging developers to always initialize their variables.
And this is the warning shot. This is the bullet that grazed across your nose. The actual issue is far far far far far more insidious than you even imagine.
Using an uninitialized value is "undefined behavior" (except for a few corner cases such as char
). Undefined behavior (or UB for short) is so insanely and completely bad for you, that you should never ever ever believe it is better than the alternative. Sometimes you can identify that your particular compiler defines the UB, and then its safe to use, but otherwise, undefined behavior is "any behavior the compiler feels like." It may do something you'd call "sane" like have an unspecified value. It may emit invalid opcodes, potentially causing your program to corrupt itself. It may trigger a warning at compile time, or the compiler may even consider it an error outright.
Or it may do nothing at all
My canary in the coal mine for UB is a case from a SQL engine that I read about. Forgive me for not linking it, I've failed to find the article again. There was a buffer overrun issue in the SQL engine when you passed a larger buffer size to a function, but only on a particular version of Debian. The bug got dutifully logged, and explored. The funny part was: the buffer overrun was checked. There was code to handle the buffer overrun in place. It looked something like this:
// move the pointers properly to copy data into a ring buffer.
char* putIntoRingBuffer(char* begin, char* end, char* get, char*put, char* newData, unsigned int dataLength)
{
// If dataLength is very large, we might overflow the pointer
// arithmetic, and end up with some very small pointer number,
// causing us to fail to realize we were trying to write past the
// end. Check this before we continue
if (put + dataLength < put)
{
RaiseError("Buffer overflow risk detected");
return 0;
}
...
// typical ring-buffer pointer manipulation followed...
}
I've added more comments in my rendition, but the idea is the same. If put + dataLength
wraps around, it will be smaller than the put
pointer (they had compile time checks to make sure unsigned int was the size of a pointer, for the curious). If this happens, we know the standard ring buffer algorithms might get confused by this overflow, so we return 0. Or do we?
As it turns out, overflow on pointers is undefined in C++. Because most compilers are treating pointers as integers, we end up with typical integer overflow behaviors, which happen to be the behavior we want. However, this is undefined behavior, meaning the compiler is allowed to do anything it wants.
In the case of this bug, Debian happened to choose to use a new version of gcc that none of the other major Linux flavors had updated to in their production releases. This new version of gcc had a more aggressive dead-code optimizer. The compiler saw the undefined behavior, and decided the result of the if
statement would be "whatever makes the code optimization best," which was an absolutely legal translation of UB. Accordingly, it made the assumption that since ptr+dataLength
can never be below ptr
without a UB pointer overflow, the if
statement would never trigger, and optimized out the buffer overrun check.
The use of "sane" UB actually caused a major SQL product to have a buffer overrun exploit that it had written code to avoid!
Never rely on undefined behavior. Ever.
bytes_read
is not changed (so kept zero), why is this supposed to be a bug? The program could still continue in a sane manner as long it does not implicitly expectsbytes_read!=0
afterwards. So it is fine sanitizers don't complain. On the other hand, whenbytes_read
is not initialized beforehand, the program won't be able to continue in a sane manner, so not initializingbytes_read
actually introduces a bug which was not there beforehand.\0
it is buggy. If it is documented not to deal with that, your calling code is buggy. If you fix your calling code to check forbytes_read==0
before calling use, then you're back to where you started: your code is buggy if you don't initializebytes_read
, safe if you do. (Usually functions are supposed to fill their out-parameters even in case of an error: not really. Quite often the outputs are either left alone or undefined.)err_t
returned bymy_read()
? If there's a bug anywhere in the example, that's it.