Newer compilers usually add more warnings, so having -Werror by default
will most likely result in a compile error when new compiler versions
are released.
MSVC: Remove "-D_SILENCE_TR1_NAMESPACE_DEPRECATION_WARNING". This is no
longer relevant, since we don't support anything older than MSVC 2015.
Disabled these warnings:
- C4005: macro redefinition (libpng's intprefix.out.tf1 is breaking on this...)
- C4091: 'typedef ': ignored on left of 'tagGPFIDL_FLAGS' when no variable is declared
- C4800: 'BOOL': forcing value to bool 'true' or 'false' (performance warning)
TODO:
- Re-enable C4800 as not-an-error?
- Figure out how to fix libpng's C4005 warnings.
The MSVC build (64-bit, at least...) now builds with no warnings.
Tested using MSVC 2022 17.6.5.
NOTE: /W4 adds a *lot* of warnings that are mostly just noise. Need to
check /W4 and selectively enable at least some of them...
Removed /wd4482. I don't think this warning is relevant anymore.
Warning fixes:
[extlib] Add (unsigned int) casts where necessary.
[librpbase] Config::ImgTypePrio_t, KeyManager::KeyData_t:
- Change `length` from unsigned int to size_t.
The struct is 16 bytes on 64-bit either way, but making it size_t
fixes some conversion warnings.
[librpbase] Hash::Process():
- crc32() returns `unsigned long`, not `uint32_t`, for some reason.
- Also, its length parameter is `unsigned int`, not `size_t`.
- KeyManager::hexStringToBytes(): Take a `size_t` length parameter.
[librpfile] scsi:
- Change cdb_len from `uint8_t` to `size_t`. Using a `uint8_t` argument
doesn't actually save any memory, since it uses 4 bytes on the stack
for 32-bit, and one register on 64-bit, regardless.
- Also, limit the maximum cdb size to 260.
[rp-download] Disable warning C4996 when calling GetVersionEx().
[librptexture] DirectDrawSurface:
- Change some `expected_size` variables from `unsigned int` to `size_t`.
[librptexture] PalmOS_Tbmp:
- Change d->bitmapTypeAddr from `off64_t` to `uint32_t`.
- Change some size variables from `unsigned int` to `size_t`.
- loadTbmp(): v3 transparency: Explicitly cast the transparency value
to uint16_t. (It's stored as a big-endian 32-bit value, but only
16 bits are used.)
- getNextTbmpAddress(): Cast addresses to `uint32_t`. PalmOS executables
are 32-bit and cannot possibly exceed 4 GB.
- FIXME: This function isn't used? (Was it ever used?)
[libromdata]
- DMGPrivate::CartType(): Make `end_offset` constexpr.
- NCCHReader: Add casts for EncSections when using sizeof().
- KeyStoreUIPrivate: binToHexStr(), verifyKeyData():
- Take a `size_t` length parameter.
- WiiUPackagePrivate: Added a parseHexBinary32() wrapper function
that returns `uint32_t` instead of `uint64_t`.
[librpbyteswap/tests]
- BitstuffTest: Cast time(nullptr) to `unsigned int`.
srand() takes `unsigned int`, not `time_t`.
[librpbase/tests]
- RpPngFormatTest: crc32()'s length parameter is `unsigned int`,
not `size_t`.
[libromdata/tests]
- SuperMagicDriveTest: zlib uses `unsigned int`, so cast array sizes
to `unsigned int` instead of changing decompress() to take `size_t`.
On Windows 10 1607+, this flag is equivalent to calling the
SetDefaultDllDirectories() function before the program starts.
We're already calling SetDefaultDllDirectories(LOAD_LIBRARY_SEARCH_SYSTEM32)
in librpsecure/win32/restrict-dll.c, but this ensures that it's set even
before the program starts.
Related changes:
- amiiboc: Use delay-loading for libfmt. Otherwise, it fails with
error code 0xC0000135.
- Added a dummy stdafx.h file, required by DelayLoadHelper.c.
- All tests: Set /DEPENDENTLOADFLAG:0xA00 because gtest cannot be
delay-loaded.
- msvc.cmake: Fix linker flags for RelWithDebInfo.
TODO:
- EXE: Show DEPENDENTLOADFLAG.
- Test svrplus.
FIx an error in the non-clang section that caused the common conformance
flags to be overwritten with the MSVC-only flags. Not sure if this will
break anything, and I can't build with MSVC at the moment...
Move "/Zc:throwingNew" into the same non-clang-cl block as the other
options.
Enable it for both debug and release if using 10.0 or later.
This fixes an issue when running package.cmd that caused
InitPropVariantFromUInt16() to not be detected correctly when using
the Windows 7 SDK. (/permissive- was only enabled for debug builds,
but MSVC builds are technically debug and release, so it got enabled
in the InitPropVariantFromUInt16() check.)
These errors occurred without this fix:
c:\program files (x86)\microsoft sdks\windows\v7.1a\include\objbase.h(239): error C2760: syntax error: unexpected token 'identifier', expected 'type specifier'
c:\program files (x86)\microsoft sdks\windows\v7.1a\include\gdiplusheaders.h(891): error C4596: 'EmfToWmfBits': illegal qualified name in member declaration
c:\program files (x86)\microsoft sdks\windows\v7.1a\include\gdiplusstringformat.h(220): error C4596: 'GetTrimming': illegal qualified name in member declaration
Even 12.0 might be too low, but 10.0 is definitely not usable anymore.
CMAKE_SYSTEM_VERSION is set to 10.0 so it will use the latest installed
Windows 10 SDK. This is needed for proper handling of /permissive-
on MSVC 2017 and later.
This makes MSVC a bit more standards-compliant, though it does require
working around some common Win32 idioms. In particular, some const_casts
are needed in order to use constant strings in various structs.
src\win32\RP_XAttrView.cpp(263,31): error C2440: '=': cannot convert from 'const wchar_t [5]' to 'LPWSTR'
src\win32\RP_XAttrView.cpp(263,21): message : Conversion from string literal loses const qualifier (see /Zc:strictStrings)
src\win32\RP_XAttrView.cpp(265,32): error C2440: '=': cannot convert from 'const wchar_t [6]' to 'LPWSTR'
src\win32\RP_XAttrView.cpp(265,21): message : Conversion from string literal loses const qualifier (see /Zc:strictStrings)
src\win32\RP_ShellPropSheetExt.cpp(890,30): error C2440: '=': cannot convert from 'const wchar_t [1]' to 'LPWSTR'
src\win32\RP_ShellPropSheetExt.cpp(890,24): message : Conversion from string literal loses const qualifier (see /Zc:strictStrings)
src\win32\config\AboutTab.cpp(1421,30): error C2440: '=': cannot convert from 'const wchar_t [6]' to 'LPWSTR'
src\win32\config\AboutTab.cpp(1421,19): message : Conversion from string literal loses const qualifier (see /Zc:strictStrings)
Our minimum supported 64-bit Windows is technically NT 5.2, which
includes both Windows Server 2003 and Windows XP x64 Edition,
so we need to disable thread-safe statics on amd64.
This partially reverts commit 648c9d8915.
([cmake] msvc.cmake: Consolidate "/guard:cf"; clean up some other checks.)
C4477: 'function' : format string 'string' requires an argument of type
'type', but variadic argument number has type 'type'
Roughly equivalent to gcc's -Werror=format-security.
The "/guard:cf" linker check was using the wrong variable, so it didn't
work correctly. Handle it in the compiler flag check instead.
Simplify the checks for "/CETCOMPAT".
Use two underscores in the "/Zc:throwingNew" check for consistency with
the other checks.
Only disable thread-safe statics on i386 for Windows XP compatibility.
We're supporting a minimum of Windows Vista for amd64, and for ARM CPUs,
the minimum is Windows 10.
svrplus is Windows only, so instead of gettext, we can use
string tables in resource files.
NOTE: Due to differences between MSVC's rc.exe and binutils's windres,
strings in the string table must be NULL-terminated.
- rc.exe ignores the explicit NULL-termination, but has an option "/n"
that automatically adds NULL terminators to strings.
- windres.exe doesn't have an equivalent to "/n", but it *does* allow
adding '\0' to strings to NULL-terminate them.
Updated msvc.cmake to add "/n" to CMAKE_RC_FLAGS.
NOTE: The '\0's in VS_VERSION_INFO doesn't actually do anything in the
resulting resource section with either rc.exe or windres.exe, so get
rid of them. I think this was mentioned on The Old New Thing before.
Renamed resource.rc to res-en_US.rc and added en_US identifiers.
Compiled executable size doesn't change much.
MSVC 2019, 32-bit release build: (both LTO and no-LTO are identical)
svrplus.exe: 114176 -> 114176 (no change)
text data bss dec hex filename
109644 2048 0 111692 1b44c svrplus.exe [before]
110650 2048 0 112698 1b83a svrplus.exe [after]
+1006 0 0 +1006 +3ee Difference
- Split debug symbols doesn't work properly due to an issue with `strip`:
error: symbols referenced by indirect symbol table entries that can't be stripped in: [library]
- Add "arm64" for M1/M2 Macs.
- zstd: "-fdeclspec" is needed for Clang due to use of __declspec() even
on non-MS compilers or OSes. Note that we have to check for both
"Clang" and "AppleClang", since Apple uses its own customized version
of LLVM/clang.
- msvc.cmake: Make sure "Clang" is quoted when STREQUAL is used.
RegKey is only used by win32/ (Win32 UI frontend), so it doesn't need to
be in libwin32common, which results in it getting exported by
romdata-2.dll.
NOTE: In theory, we can remove the HINSTANCE parameter from
RegisterComObject() now, since RegKey is always statically-linked into
rom-properties.dll, but we're better off keeping it there.
[doc/abi] RegKey has been removed from the Windows version.
clang-cl prints a warning, not an error, for these, so it doesn't get
detected by CHECK_CXX_COMPILER_FLAG():
clang-cl : warning : argument unused during compilation: '/Zc:externC' [-Wunused-command-line-argument]
clang-cl : warning : argument unused during compilation: '/Zc:noexceptTypes' [-Wunused-command-line-argument]
- /Zc:inline
- /Zc:__cplusplus
- /Zc:externC
- /Zc:noexceptTypes
- /Zc:rvalueCast
- /Zc:ternary
No code changes were needed, since gcc implements all of these anyway
as far as I know.
clang-cl defaults to /Zc:throwingNew internally, but it prints a warning
if the parameter is specified:
clang-cl : warning : argument unused during compilation: '/Zc:throwingNew'
[-Wunused-command-line-argument]
KDE Frameworks 5.85.0 enables this by default. It reduces startup time
by pre-linking global function symbols.
Also, filter out '=' from variable names. This broke the caching of
-Werror=return-type.
msvc.cmake: Copy the '=' change here.
I tested a release build and it saved a total of 10,752 bytes.
On the other hand, it had a rather huge maintenance overhead, since I had
to ensure that all extlibs had __cdecl set up in the headers properly,
and this had to be redone on every update.
The i386 build of LZ4 on AppVeyor was failing in tests because of missing
stdcall symbols. I decided not to bother adding stdcall support to LZ4
and simply revert stdcall entirely.
I decided to try building rom-properties with MSVC 2019's ARM compilers,
and it seems to build properly. I can't test it at the moment, though.
CMake changes:
- ${CMAKE_SYSTEM_PROCESSOR} is always set to the host CPU. Check
_MSVC_C_ARCHITECTURE_FAMILY to determine the real CPU, then set
CMAKE_SYSTEM_PROCESSOR accordingly.
- Enable /EHsc if it isn't set already. It's set by CMake for i386
and amd64, but not for ARM or ARM64.
- Minimum subsystem version for ARM and ARM64 is 6.2. (Windows 8)
- Added some Win32 libraries that are included by default on i386 and
amd64, but not on ARM or ARM64.
- Reference: https://pete.akeo.ie/2017/05/compiling-desktop-arm-applications-with.html
Source changes:
- DelayLoadHelper.c, rp-config.c: Added subdirectory names for
ia64, ARM, and ARM64.
- TODO: Consolidate this into a common header file?
FIXME: Maybe we should use RP_LINKER_FLAGS_WIN32_EXE and
RP_LINKER_FLAGS_CONSOLE_EXE on MSVC. (We're using this on gcc.)
Not sure why I used one method or the other...
C4024: 'function': different types for formal and actual parameter n
C4047: 'function': 'parameter' differs in levels of indirection from 'argument'
NOTE: /sdl doesn't do this.
Thread-safe static initialization requires Vista or later, since XP
doesn't have enough TLS slots.
NOTE: We need to make sure that we use pthread_once() for one-time init
instead of using a static initializer.
Release build sizes: (MSVC 2017)
- rom-properties.dll: 655,360 -> 676,864 (+21,504 bytes)
- rpcli.exe: 480,768 -> 496,128 (+15,360 bytes)
It's not that much overhead, so I'll keep it enabled for all builds.
