Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes to build with MSVC with /W4 #2664

Closed
wants to merge 3 commits into from

Conversation

cwoffenden
Copy link
Contributor

Updating to 1.5.0 no longer builds with MSVC's warning level 4 due to "potentially uninitialized" local vars. It's a false positive, since each use is wrapped in a test for if (dictMode == ZSTD_dictMatchState). The quick fix is to globally ignore the warnings, silencing these:

error C2220: the following warning is treated as an error
warning C4701: potentially uninitialized local variable 'nextSeqStore' used
warning C4701: potentially uninitialized local variable 'ddsIdx' used
warning C4701: potentially uninitialized local variable 'ddsExtraAttempts' used
warning C4701: potentially uninitialized local variable 'dmsTag' used
warning C4701: potentially uninitialized local variable 'dmsTagRow' used
error C4703: potentially uninitialized local pointer variable 'dmsTagRow' used
warning C4701: potentially uninitialized local variable 'dmsRow' used
error C4703: potentially uninitialized local pointer variable 'dmsRow' used

It's not a comfortable warning to globally silence so the alternative would be to locally silence them (which is preferable to setting them to anything just to make the compiler happy).

@cwoffenden
Copy link
Contributor Author

This is doing the same as #2654 just in a different way.

@felixhandte
Copy link
Contributor

felixhandte commented Jun 7, 2021

I've just merged the other PR. So I'm going to close this in the expectation that the underlying issue is fixed.

Thanks for the contribution though!

@felixhandte felixhandte closed this Jun 7, 2021
@cwoffenden cwoffenden deleted the msvc-w4-fixes branch June 7, 2021 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants