Fix various compiler warnings on MSVC

[adrido]

I tried to avoid casts wherever it was possible, but for some cases it wasn't. There are many C-like casts, maybe you should consider to replace them some day.

The only warnings that occur now are from leveldb or are about using c++11 functions instead of c++99 (even occurs in Microsofts libraries :laughing: )

[Rogier-5]

The only warnings that occur now are from leveldb or are about using c++11 functions instead of c++99 (even occurs in Microsofts libraries :laughing: )

Enable c++11 mode (or c++0x). That should remove those warnings...

After all, the code does use a number of c++0x or c++11 language and library features.

At the moment, I usually compile in c++11 mode (I made the build environment enable that if available), and from time to time , I test compiling in c++0x mode. As I know some c++0x features are used, I made the Linux build environment actually refuse to compile if anything less is detected...

There are many C-like casts, maybe you should consider to replace them some day.

I should... Do they generate warnings on msvc ?

[Rogier-5]

And BTW: what are the leveldb warnings about ?

[adrido]

The warnings that still exist are the following:

1>..\TileGenerator.cpp(2425): warning C4996: 'fopen': This function or variable may be unsafe. Consider using fopen_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
1>  C:\Program Files (x86)\Windows Kits\10\Include\10.0.10240.0\ucrt\stdio.h(205): note: Siehe Deklaration von "fopen"
1>..\TileGenerator.cpp(2428): warning C4996: 'strerror': This function or variable may be unsafe. Consider using strerror_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
1>  C:\Program Files (x86)\Windows Kits\10\Include\10.0.10240.0\ucrt\string.h(178): note: Siehe Deklaration von "strerror"
1>..\mapper.cpp(203): warning C4996: 'fopen': This function or variable may be unsafe. Consider using fopen_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
1>  C:\Program Files (x86)\Windows Kits\10\Include\10.0.10240.0\ucrt\stdio.h(205): note: Siehe Deklaration von "fopen"
1>..\mapper.cpp(208): warning C4996: 'getenv': This function or variable may be unsafe. Consider using _dupenv_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
1>  C:\Program Files (x86)\Windows Kits\10\Include\10.0.10240.0\ucrt\stdlib.h(1183): note: Siehe Deklaration von "getenv"
1>wingetopt\src\getopt.c(348): warning C4996: 'getenv': This function or variable may be unsafe. Consider using _dupenv_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
1>  C:\Program Files (x86)\Windows Kits\10\Include\10.0.10240.0\ucrt\stdlib.h(1183): note: Siehe Deklaration von "getenv"

If I can trust this site fopen_s is valid C++11. But changing to this would break compatibility with older build environments that does not support it. So I would keep it as it is, and keep the warning enabled. Disable warnings is IMO a bad idea.

There are many C-like casts, maybe you should consider to replace them some day.

I should... Do they generate warnings on msvc ?

No, they doesn't, but they generating warnings in my head :smile:

The leveldb warnings only occur in x64 build mode and reads as follows:

1>c:\users\adrian\documents\minetest-mapper-cpp\packages\leveldb.1.16.0.5\lib\native\src\db/skiplist.h(357): warning C4312: "reinterpret_cast": Konvertierung von "int" in größeren Typ "void *"
1>  c:\users\adrian\documents\minetest-mapper-cpp\packages\leveldb.1.16.0.5\lib\native\src\db/skiplist.h(334): note: Bei der Kompilierung der Klassen-template der "void leveldb::SkipList<const char *,leveldb::MemTable::KeyComparator>::Insert(const Key &)"-Memberfunktion
1>          with
1>          [
1>              Key=const char *
1>          ]
1>  c:\users\adrian\documents\minetest-mapper-cpp\packages\leveldb.1.16.0.5\lib\native\src\db/memtable.cc(105): note: Siehe Verweis auf die Instanziierung der gerade kompilierten Funktions-template "void leveldb::SkipList<const char *,leveldb::MemTable::KeyComparator>::Insert(const Key &)".
1>          with
1>          [
1>              Key=const char *
1>          ]
1>  c:\users\adrian\documents\minetest-mapper-cpp\packages\leveldb.1.16.0.5\lib\native\src\db/memtable.h(82): note: Siehe Verweis auf die Instanziierung der gerade kompilierten Klassen-template "leveldb::SkipList<const char *,leveldb::MemTable::KeyComparator>".
1>C:\Users\Adrian\Documents\minetest-mapper-cpp\packages\LevelDB.1.16.0.5\lib\native\include\leveldb/slice.h(97): warning C4267: "Initialisierung": Konvertierung von "size_t" nach "int", Datenverlust möglich
1>C:\Users\Adrian\Documents\minetest-mapper-cpp\packages\LevelDB.1.16.0.5\lib\native\include\leveldb/slice.h(97): warning C4267: "Initialisierung": Konvertierung von "size_t" nach "const int", Datenverlust möglich

Maybe its because its probably a worse port of leveldb. The author of the leleldb windows port did not give an answer to my bug-report of snappy

Ill apply your recommendations and push it again.

[Rogier-5]

If I can trust this site fopen_s is valid C++11. But changing to this would break compatibility with older build environments that does not support it. So I would keep it as it is, and keep the warning enabled.

From a programming point of view, there seems absolutely no good reason to avoid fopen, or deprecate it. From what I read, fopen_s has a bit different behavior, and more options. The way I see it, if you don't need all that, fopen is perfectly fine. I don't understand why Microsoft went on a crusade to eliminate fopen (and friends), and I can only guess as to why the C++ committee included Microsoft's extensions in C++11. Note that they are optional in C++11 ! I suppose the fact that they are optional really explains it all...

Disable warnings is IMO a bad idea.

As far as I'm concerned, it depends on the type of warning, the type of code, the amount of work involved in fixing them, and the benefit of fixing them.

In this case of Microsoft choosing to deprecate an interface and issuing warnings, while deprecating that interface is not even being discussed in the c++ standard committe (except probably by Microsoft-employed members), and the alternative interface being only an optional part of the standard, I'd choose to disable the warning.

As far as Minetestmapper is concerned, I think I would personally favor disabling these warnings when compiling a release version, so that people compiling minetestmapper themselves are not distracted by those messages that for the moment, are here to stay. I am perfectly fine with having them enabled when compiling a debug version. Is that doable ? Would you object to having those warnings disabled when compiling a release version ?

The leveldb warnings only occur in x64 build mode and reads as follows:

I understand they only happen when compiling leveldb, not when compiling minetestmapper ?