Compilation warnings with GCC5

Bug #1455763 reported by Hans Joachim Desserud
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
widelands
Fix Released
Undecided
Unassigned

Bug Description

After some initial trouble (bug 1432339) I've successfully built Widelands with GCC5, more specifically 5.1.1-5.

I've attached a list of the warnings I got while compiling. I know we were warning-free with GCC at one point and it would be nice to get there again. It makes it much easier to spot when new warnings are introduced and fix potentially dodgy behaviour in the code.

I looked at some of them, particularly the argument type ones, but didn't really get anywhere.

Related branches

Revision history for this message
Hans Joachim Desserud (hjd) wrote :
Revision history for this message
GunChleoc (gunchleoc) wrote :
Download full text (4.4 KiB)

This one is from third party code and has been there since we started using Eris:

../../third_party/libthird_party_eris.a(loslib.c.o): In function `os_tmpname':
/home/debian/widelands/src/third_party/eris/loslib.c:108: warning: the use of `tmpnam' is dangerous, better use `mkstemp'
In file included from /home/debian/widelands/src/scripting/eris.h:25:0,
                 from /home/debian/widelands/src/scripting/persistence.cc:27:
/home/debian/widelands/src/third_party/eris/eris.h:145:38: warning: redundant redeclaration of ‘int luaopen_eris(lua_State*)’ in same scope [-Wredundant-decls]
 LUA_API int luaopen_eris(lua_State* L);
                                      ^

The molog messages like the one below won't fix - the compiler can't handle the polymorphic pointer type used by %p in sprintf. Both Tibor and me tried to fix these and did the research.

/home/debian/widelands/src/logic/bob.cc: In member function ‘virtual void Widelands::Bob::log_general_info(const Widelands::EditorGameBase&)’:
/home/debian/widelands/src/logic/bob.cc:1002:30: warning: format ‘%p’ expects argument of type ‘void*’, but argument 3 has type ‘Widelands::Player*’ [-Wformat=]
  molog("Owner: %p\n", m_owner);
                              ^

This leaves us with the following warnings to try and fix:

/home/debian/widelands/src/map_io/map_players_messages_packet.cc: In member function ‘void Widelands::MapPlayersMessagesPacket::read(FileSystem&, Widelands::EditorGameBase&, bool, Widelands::MapObjectLoader&)’:
/home/debian/widelands/src/map_io/map_players_messages_packet.cc:80:41: warning: format ‘%u’ expects argument of type ‘unsigned int’, but argument 3 has type ‘Widelands::Message::Type’ [-Wformat=]
        begin->second->body ().c_str());
                                         ^
/home/debian/widelands/src/map_io/map_players_messages_packet.cc:80:41: warning: format ‘%u’ expects argument of type ‘unsigned int’, but argument 8 has type ‘Widelands::Message::Status’ [-Wformat=]
/home/debian/widelands/src/map_io/map_players_view_packet.cc: In member function ‘void Widelands::MapPlayersViewPacket::write(FileSystem&, Widelands::EditorGameBase&, Widelands::MapObjectSaver&)’:
/home/debian/widelands/src/map_io/map_players_view_packet.cc:1145:12: warning: suggest parentheses around operand of ‘!’ or change ‘&’ to ‘&&’ or ‘!’ to ‘~’ [-Wparentheses]
       if (!bl_seen & (f_everseen | bl_everseen))
            ^
/home/debian/widelands/src/map_io/map_players_view_packet.cc:1147:12: warning: suggest parentheses around operand of ‘!’ or change ‘&’ to ‘&&’ or ‘!’ to ‘~’ [-Wparentheses]
       if (!br_seen & (f_everseen | br_everseen))
            ^
/home/debian/widelands/src/map_io/map_players_view_packet.cc:1149:12: warning: suggest parentheses around operand of ‘!’ or change ‘&’ to ‘&&’ or ‘!’ to ‘~’ [-Wparentheses]
       if (!r_seen & (f_everseen | r_everseen))

In file included from /home/debian/widelands/src/logic/instances.cc:20:0:
/home/debian/widelands/src/logic/instances.h: In constructor ‘Widelands::MapObject::MapObject(const Widelands::MapObjectDescr*)’:
/home/debian/widelands/src/logic/instances.h:366:27: warning: ‘Widelands::MapObject::m_logsink’ will be initiali...

Read more...

Revision history for this message
Hans Joachim Desserud (hjd) wrote :

Regarding the eris-related ones:
the use of `tmpnam' is dangerous, better use `mkstemp'

I actually know what's causing this, but I'm not quite sure how to fix it. If you build eris (or lua) as a stand-alone, you run `make` with some option, for instance `make linux`, `make posix` or `make generic`. This says a bit about the platform you're building for and sets various flags/ifdefs used in the code. For instance, if you specify linux, it will use `mkstemp` as the warning says.

But then why are we still getting the warning, you say? Because it those flags are only set when the build command (`make`) is called with an option. When Widelands invokes the building it simply does `cmake .. && make` (simplified), so no options for eris. I am not sure how we could somehow pass an extra option along for just the eris files and more importantly how to pick the proper one (remember, this various from platform to platform). I don't know if there's an easy fix for this one...

The other one though, warning about redundant declaration, looks like we specify something in the code without actually needing to do so.

Your other comments sounds ok. A shame about the molog ones though :/

GunChleoc (gunchleoc)
tags: added: cleanups
removed: gcc
Revision history for this message
GunChleoc (gunchleoc) wrote :

All fixed in r8383 except for the eris one, which is triggered by 3rd-party code and that we can live with.

Changed in widelands:
milestone: none → build20-rc1
status: Triaged → Fix Committed
Revision history for this message
Hans Joachim Desserud (hjd) wrote :

Nice :D Great to see a warning-free build.

(we can certainly live with the third-party ones, which we cannot really affect much either way)

Revision history for this message
GunChleoc (gunchleoc) wrote :

Fixed in build20-rc1

Changed in widelands:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Bug attachments

Remote bug watches

Bug watches keep track of this bug in other bug trackers.