OutpostUniverse / OP2Utility

C++ library for working with Outpost 2 related files and tasks.
MIT License
4 stars 0 forks source link

New Unitialized variable warning on MSVC #217

Closed Brett208 closed 5 years ago

Brett208 commented 5 years ago

I recently updated to a new version of Visual Studio 17. After that, when I compiled the code, I got the following warnings:

Warning C26495 Variable 'Archives::ClmFile::ClmHeader::fileVersion' is uninitialized. Always initialize a member variable (type.6). OP2Archive \op2archive\op2utility\src\archives\clmfile.h 37

Warning C26495 Variable 'Archives::ClmFile::ClmHeader::packedFilesCount' is uninitialized. Always initialize a member variable (type.6). OP2Archive \op2archive\op2utility\src\archives\clmfile.h 37

Warning C26495 Variable 'Archives::ClmFile::ClmHeader::unknown' is uninitialized. Always initialize a member variable (type.6). OP2Archive \op2archive\op2utility\src\archives\clmfile.h 37

Warning C26495 Variable 'Archives::ClmFile::ClmHeader::waveFormat' is uninitialized. Always initialize a member variable (type.6). OP2Archive \op2archive\op2utility\src\archives\clmfile.h 37

https://docs.microsoft.com/en-us/visualstudio/code-quality/c26495?f1url=https%3A%2F%2Fmsdn.microsoft.com%2Fquery%2Fdev15.query%3FappId%3DDev15IDEF1%26l%3DEN-US%26k%3Dk(C26495)%26rd%3Dtrue&view=vs-2017

Following the link from the MSDN help above leads to a document authored by Herb Sutter and Bjarne Stroustrup. Interesting read.

It looks like Visual Studio has the ability to turn on different checks for warnings that are outlined on the linked GitHub page.

Brett

DanRStevens commented 5 years ago

... rather than a zstring. French accent optional.

:smirk:


I see, it seems to be complaining about the empty constructor which doesn't initialize any fields. I can see why that might be cause for a warning. Though in terms of the use of this class, we don't really want to initialize fields when we allocate space for the struct since it will (typically) be read in from disk by overwriting the objects memory location.

Technically, to overwrite memory like that, we should be using an aggregate type. An aggregate type should not have a user defined constructor. We have 2 constructors. One is a dummy one, which does nothing, and results in that warning. It only exists to allow default construction, which is disabled by the second constructor:

ClmHeader(WaveFormatEx waveFormatEx, uint32_t packFileCount);

For us to read from the file as we do, and remain standards compliant (or as close as possible?), we should remove the constructors. The second constructor could potentially become a static method.

Brett208 commented 5 years ago

I think we can close this now as the branch fixing it was merged.