Fix unitialized variable in zstream redup command

[behlendorf]

Motivation and Context

Issue #10241 .

Description

Fix uninitialized variable in zstream redup command. The compiler
may determine the 'stream_offset' variable can be uninitialized
because not all rdt_lookup() exit paths set it. This should never
happen in practice as documented by the assert, but initialize it
regardless to resolve the warning.

How Has This Been Tested?

Locally compiled.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Performance enhancement (non-breaking change which improves efficiency)
• Code cleanup (non-breaking change which makes code smaller or more readable)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation (a change to man pages or other documentation)

Checklist:

• My code follows the ZFS on Linux code style requirements.
• I have updated the documentation accordingly.
• I have read the contributing document.
• I have added tests to cover my changes.
• I have run the ZFS Test Suite with this change applied.
• All commit messages are properly formatted and contain Signed-off-by.

[ahrens]

Sorry about that, thanks for fixing. Is this check part of the automated builds? If so, maybe we can turn these warnings into errors, so that we'll notice them when the PR is still open.

[behlendorf]

I was wondering myself why the CI didn't catch it. We do convert all warnings to fatal errors when building in the CI. It looks like this might have been missed because none of the bot were running a new enough version of gcc. There seems to be no warning on older versions.

[behlendorf]

Following up on why the CI missed this. It turns out the reason we didn't catch this is precisely because we mainly build with debugging enabled. When --enable-debug is specified gcc recognizes the path where stream_offset doesn't get initialized as fatal and thus it can't be uninitialized and there's no warning. Conversely, we do see the warning in production builds, but since it's a production build the warning isn't promoted to an error and the build completes successfully.

[Hi-Angel]

Following up on why the CI missed this. It turns out the reason we didn't catch this is precisely because we mainly build with debugging enabled. When --enable-debug is specified gcc recognizes the path where stream_offset doesn't get initialized as fatal and thus it can't be uninitialized and there's no warning. Conversely, we do see the warning in production builds, but since it's a production build the warning isn't promoted to an error and the build completes successfully.

That's a detailed analysis. I think it might be worth noting that the more optimizations enabled, the more likely compiler to produce various warnings. It is irrelevant to debug-codepaths vs release-codepaths stuff, like the one you stumbled upon here. Rather it's just how code analyzer works. IIRC it is the same for both Clang and GCC. I think it might be less visible with C code though (I've seen the most of such discrepancy with C++). Offhand I can't find reports on this in my browser history, but I can try to find examples if anybody wants.

That is just something to bear in mind, so not to spend time trying to figure out why release build produce a warning that is missing in debug one. Perhaps you may want to enable a -O3 buiding pass in CI, maybe even with -flto.