Fix Clang Compile Warnings/Errors (and Typos)

[geometrian]

Clang (e.g. 14.0.5) issues compile errors for several issues when compiled with strict settings. Specifically, the capitalization of the Windows header file is incorrect, NULL is accidentally used in some places, and there are some implicit casts and signedness comparisons that should be clarified.

There are also a lot of typos both in the comments and in error messages.

This PR fixes all identified issues, separated into two commits which can be cherrypicked as the repository manager desires.

[syoyo]

@geometrian Thanks! I didn't realized there are so much typos in comments 😅

For header filename capitalization, IIRC it was an annoying issue of the combination of compiler front-end's (virtual) filesystem and underlying OS' filesystem(especially emerges when cross-compiling). For example MinGW uses all lower-case header filenames. So I gave up using Capitalized header file name.

• https://reviews.llvm.org/D21113
• nonportable-include-path warning on case-insensitive systems commercialhaskell/stack#3918

so possible solution may be...

#ifdef __MINGW32__ // mingw gcc, llvm-mingw(clang)
#include "windows.h"
#elif defined(_MSC_VER) || [clang-cl and clang on case-sensitive filesystem(how to determine?)]
#include "Windows.h"
#else
#include "windows.h"
#endif

[geometrian]

Yah; I noticed it failed on MinGW when the CI check failed, and pushed another commit up, which fixes it. It works similarly to what you just now suggested: it normally uses <Windows.h>, while on MingGW it uses <windows.h>. The internet tells me there are still other cases if it's supposed to run on mobile or ancient Windows, but for now at least this is better than before and passes all tests. Sigh, MS 😩

Another solution to consider for Clang particularly would be to temporarily disable the warning for #includeing that one file, or to get the functionality some other way (e.g. C++17 std::filesystem; I know you're targeting C++11, but 2022 > 2011, sooo).

[syoyo]

@geometrian Thanks! Merged!

Another solution to consider for Clang particularly would be to temporarily disable the warning for #includeing that one file

Since tinygltf uses few WIn32 API, how about declare it explicitly in tiny_gltf.h and do not include windows.h header file?

[geometrian]

how about declare it explicitly in tiny_gltf.h and do not include windows.h header file?

Not sure what you mean? Like, making the user include <Windows.h> themselves? Or, forward-declaring the Win32 API functions? I generally feel like both options could be potentially surprising, but could work. I personally think the current method is good (maybe a few tweaks down the road, even robust to weird MS platforms), especially if stuff like moving implementation to the ".cc" file or using a modern language standard aren't on the table.

temporarily disable the warning for #includeing that one file

Just to clarify, the idea there was to make Clang be quiet about the nonportability of the filename. The warning is correct, but not fixable by anyone for this particular file now that MS has defined different cases, nor relevant to actual portability given that it's a platform-specific header. Disabling the warning to ignore fundamental brokenness is just as good in my head as patching around the brokenness.

[syoyo]

forward-declaring the Win32 API functions?

Yes I meant it: e.g.

extern int MultiByteToWideChar(...)
#define CP_UTF8 ...

temporarily disable the warning for #includeing that one file

This would be also an solution. Additional PR to suppress clang warning for #include <Windows.h> appreciated!

[geometrian]

Err, the solutions are orthogonal; only one is required. If you really wanted to, you could do both:

#ifdef __clang__
	#pragma clang diagnostic push
	#pragma clang diagnostic ignored "-Wnonportable-system-include-path"
#endif
//(Windows.h include stuff here.)
#ifdef __clang__
	#pragma clang diagnostic pop
#endif

but again there's no point, since the header is now being included more portably.

If you're just using it for MultiByteToWideChar though, one should probably change all this to #include <stringapiset.h> instead of <Windows.h>. Perhaps the capitalization of that is more reliable, even besides including far less stuff.