OpenTechEngine / OpenTechBFG

Engine based on (RB) Doom 3 BFG aiming to allow the creation of standalone games
Other
84 stars 11 forks source link

fixed compilation error with gcc version 7.1.1 #136

Closed damiel closed 6 years ago

damiel commented 7 years ago

Pretty weird issue with gcc 7.1.1 on arch linux, took me a while to figure out.

The stuff regading dmap is especially weird, why the hell would we use stdlib print calls in there?

BielBdeLuna commented 7 years ago

I think this is due the dmap code being a port from the doom3 engine where things where handled in a older fashion way.

damiel commented 7 years ago

@BielBdeLuna Thanks for the explanation, this makes sense.

DanielGibson commented 7 years ago

The stuff regading dmap is especially weird, why the hell would we use stdlib print calls in there?

it doesn't, and your fix probably breaks it. It didn't call printf() which prints to stdout, but sprintf(), which puts the formatted string into the buffer given as first argument (path in this case). If you can tell us the compiler error message, maybe we can figure out a proper fix :)

I'm not sure about the idStrStatic fixes either - if they explicitly called idStr::operator=() (which should be ok, as idStrStatic is derived from idStr), they probably meant to do that. Again, the actual compiler message would be helpful to figure out what goes wrong there.

damiel commented 7 years ago

Here are the error messages regarding idStr/idStrStatic: https://bpaste.net/show/8a19c7480bbf

And here the error messages regarding dmap: https://bpaste.net/show/f6b3abbcc533

DanielGibson commented 7 years ago

std::sprintf() should probably be replaced with variant of snprintf(). idStr::snPrintf() should work (note that it's a static method that operates on a char buffer and not on an idStr)

The operator=() thing is kinda fishy, as idStr only overloads operator= for idStr and const char*, not bool, float etc. I think that it (before gcc7) uses operator=(const idStr& text) and implicitly casts bool, float etc to idStr with idStr's constructors for these types. Long story short: Try replacing idStr::operator=( b ); with idStr::operator=( idStr( b ) ); (and similar for the other cases)

damiel commented 7 years ago

Okay, i adjusted the idStr stuff now with the solution you suggested, i'll figure out the rest regarding dmap.

damiel commented 6 years ago

Once the build is done and you are okay with this solution, may i squash the commits into one?

DanielGibson commented 6 years ago

this looks better; for the sprintf()-fixes you could even use sizeof(path) instead of hardcoding 1024, in case someone wants to change paths size later.

squashing the commits sounds like a good idea :)

damiel commented 6 years ago

Done, thanks alot for your feedback!

shmerl commented 6 years ago

Why not use ::snprintf ?

shmerl commented 6 years ago

By the way, I'm also getting a ton of sprintf errors in doomclassic/doom/st_stuff.cpp and wi_stuff.cpp.

For example:

.../build/RBDOOM-3-BFG/doomclassic/doom/wi_stuff.cpp: In function ‘void WI_loadData()’:
.../build/RBDOOM-3-BFG/doomclassic/doom/wi_stuff.cpp:1502:6: error: ‘%2.2d’ directive writing between 2 and 10 bytes into a region of size 4 [-Werror=format-overflow=]

I'm building the source from RobertBeckebans/RBDOOM-3-BFG though.

BielBdeLuna commented 6 years ago

there shouldn't be any reference to doom classic in OTE

DanielGibson commented 6 years ago

Why not use ::snprintf ?

it's not properly supported in MSVC

DanielGibson commented 6 years ago

I guess rbdoom3bfg could (should) use this fix, but might need some more work for doom classic

shmerl commented 6 years ago

@DanielGibson: There actually already exists a merge request with such fixes.

shmerl commented 6 years ago

It still happens on the master by the way, for example:

[ 65%] Building CXX object neo/CMakeFiles/OpenTechEngine.dir/d3xp/MultiplayerGame.cpp.o
.../build/OpenTechBFG/neo/d3xp/MultiplayerGame.cpp: In member function ‘const char* BFG::idMultiplayerGame::GameTime()’:
.../build/OpenTechBFG/neo/d3xp/MultiplayerGame.cpp:671:13: error: ‘%i’ directive writing between 1 and 8 bytes into a region of size between 3 and 13 [-Werror=format-overflow=]
 const char* idMultiplayerGame::GameTime()
             ^~~~~~~~~~~~~~~~~
.../build/OpenTechBFG/neo/d3xp/MultiplayerGame.cpp:671:13: note: directive argument in the range [-4294940, 4294943]
.../build/OpenTechBFG/neo/d3xp/MultiplayerGame.cpp:721:15: note: ‘sprintf’ output between 5 and 22 bytes into a destination of size 16
   std::sprintf( buff, "%i:%i%i", m, t, s );
   ~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1plus: some warnings being treated as errors
neo/CMakeFiles/OpenTechEngine.dir/build.make:4502: recipe for target 'neo/CMakeFiles/OpenTechEngine.dir/d3xp/MultiplayerGame.cpp.o' failed
make[2]: *** [neo/CMakeFiles/OpenTechEngine.dir/d3xp/MultiplayerGame.cpp.o] Error 1
CMakeFiles/Makefile2:1479: recipe for target 'neo/CMakeFiles/OpenTechEngine.dir/all' failed
make[1]: *** [neo/CMakeFiles/OpenTechEngine.dir/all] Error 2
Makefile:151: recipe for target 'all' failed
make: *** [all] Error 2