Merge lp:~mxsscott/widelands/warnings into lp:widelands

[Mark Scott]

Fix or suppress GCC warnings visible when compiling with GCC 4.7.2

Specifically:
 * compile_diagnostics.h: Introduce macros GCC_DIAG_OFF and GCC_DIAG_ON that disable and then restore reporting of warnings and errors. Used to disable a certain warning for a specific line of code on GCC 4.6+, and to silence warnings in Pluto and Minizip.

 * gl_surface.cc: Asserts that will never be false because variable is unsigned.
 * panel.cc: Always-true asserts due to unsigned type.

 * editorinteractive.cc: Unused variable
 * listselect.cc: Unused variable
 * waresdisplay.cc: Unused variable
 * waresqueuedisplay.cc: Unused variable
 * graphic.cc: Unused variable and parameter and code that would never be executed.
 * graphic.h: Unused parameter. Reorder members to match constructor initialization list
 * ui_fsmenu/base.cc: Unused variable. pull_section() does have side effects, but in this case it is irrelevant as gr_x() and gr_y() in the initializer list will have the same impact. Suspect this existed before gr_x() and gr_y() were put into the initializer.

 * sdl_surface.cc: cast struct initializers appropriately.
 * rt_render.cc: cast appropriately to avoid signed/unsigned comparison.
 * Question existence / correctness of method - seems to be very similar to Rect::contains() but very slightly different in handling of bottom & right edge.
 * Comment out render() parameter name (unused parameter).
 * Provide initialisation of all struct members.

 * sdl_ttf_font_impl.cc: Change type of variable to avoid signed/unsigned comparison.

 * disk_filesystem.cc: Be specific about the lines to ignore warnings on.

 * scripting/pluto.cc: Suppress warnings about unused-variables (at top of file)

 * minizip/unzip.cc: Suppress warnings about old-style-cast (above the non-modification notice)
 * internet_gaming.cc: Suppress warning about old-style-cast
 * internet_lobby.cc: Suppress warning about old-style-cast
 * network_lan_promotion.cc: Suppress warning about old-style-cast
 * netsetup_lan.cc: Suppress warning about old-style-cast

 * nethost.cc: Add deletion of 'tips' variable, which (a) fixes a memory leak and (b) silences a warning about an unused variable (it isn't really unused, since it registers itself with the loaderUI in its constructor)

 * s2map.cc: Use of setiosflags() instead of hex() and dec() silences the sign promotion warnings

[Hans Joachim Desserud (hjd)]

Nice to see someone tackling the remaining warnings. :)

I leave reviewing the code to someone better suited for the task, but here's a few comments:

Could you elaborate a bit more on the warnings which are silenced? For minizip and similar I fully understand since we don't want to diverge from upstream (and ideally shouldn't need to include them), however I wonder about the rest.

Regarding the silencing macro, it seems nice and straight-forward to use. However, the original (http://dbp-consulting.com/tutorials/SuppressingGCCWarnings.html) doesn't seem to mention a license nor terms of use for it. Any thoughts on this? At any rate I assume it would be considered a derivate work of the original, which could cause all sorts of problems. Maybe I'm overreacting a bit, but I would like to see that clarified a bit more.

[SirVer (sirver)]

I only reviewed the code from the technical point of view and from there it looks fine. I am also not concerned about the individual disabling of the warnings in the code - it was all clear to me when I reviewed it.

About the copyright issue - the text seems to have been written/dublicated into the boost wiki as a coding reference for the boost libraries [1]. I therefore feel they are basically part of the c++ folklore and therefore free to use - and we give credit in the code. I am not a lawyer, but as long as no one complains, I think we are golden.

https://svn.boost.org/trac/boost/wiki/Guidelines/WarningsGuidelines

[Mark Scott (mxsscott)]

In terms of warnings silenced rather than fixed through code change, this was only done where no code change was desirable (the minizip and lua code), or possible (macros from headers outside of Widelands, e.g. GLEW_VERSION_1_4, htons(), ioctl-related constants).

All other warnings were fixed through code change.

Here is a thread from the original author, Jonathan Wakely, who is referenced on the Boost page and on the page I referenced in code:

http://gcc.gnu.org/ml/gcc-help/2011-01/msg00135.html

    > Also, can I steal, with attribution of course, GCC_DIAG_OFF/ON(x)?
    Certainly.

The main difference between that GCC_DIAG_OFF/ON and the one I committed was he has:
  GCC_DIAG_OFF(old-style-cast)

I have:
  GCC_DIAG_OFF("-Wold-style-cast")

Why? My version doesn't get complained at by the CodeCheck (it complained about lack of a space after an operator)

[Hans Joachim Desserud (hjd)]

Awesome.

> only done where no code change was desirable (..) or possible

Fair enough. I won't really expect anyone to track down and fix issues triggered by third party software.

>Here is a thread from the original author, Jonathan Wakely,

Nice. Based on the attribution link, it did seem like the goal was to share the macro with anyone interested, but it is nice to have it explicitly.

Thanks again for your work on the warnings.

=== added file 'src/compile_diagnostics.h'
--- src/compile_diagnostics.h	1970-01-01 00:00:00 +0000
+++ src/compile_diagnostics.h	2013-01-06 13:38:25 +0000
@@ -0,0 +1,41 @@
+/*
+ * Copyright (C) 2013 by the Widelands Development Team
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ *
+ */
+#ifndef COMPILE_DIAGNOSTICS_H
+#define COMPILE_DIAGNOSTICS_H
+
+/* Macros for disabling GCC warnings and errors
+ * From http://dbp-consulting.com/tutorials/SuppressingGCCWarnings.html and
+ * slightly modified to remove support entirely for GCC < 4.6 because we'll
+ * use in the middle of functions.
+ */
+#if ((__GNUC__ * 100) + __GNUC_MINOR__) >= 406
+# define GCC_DIAG_STR(s) #s
+# define GCC_DIAG_JOINSTR(x, y) GCC_DIAG_STR(x ## y)
+# define GCC_DIAG_DO_PRAGMA(x) _Pragma (#x)
+# define GCC_DIAG_PRAGMA(x) GCC_DIAG_DO_PRAGMA(GCC diagnostic x)
+# define GCC_DIAG_OFF(x) GCC_DIAG_PRAGMA(push) \
+         GCC_DIAG_PRAGMA(ignored x)
+# define GCC_DIAG_ON(x) GCC_DIAG_PRAGMA(pop)
+#else
+# define GCC_DIAG_OFF(x)
+# define GCC_DIAG_ON(x)
+#endif
+
+
+#endif

=== modified file 'src/editor/editorinteractive.cc'
--- src/editor/editorinteractive.cc	2012-12-14 20:09:35 +0000
+++ src/editor/editorinteractive.cc	2013-01-06 13:38:25 +0000
@@ -128,10 +128,8 @@
 		if (Widelands::Coords const sp = map.get_starting_pos(p)) {
 			const IPicture* pic = g_gr->imgcache().load(PicMod_Game, fname);
 			assert(pic);
-			uint32_t w = pic->get_w();
-			uint32_t h = pic->get_h();
 			map.overlay_manager().register_overlay
-			(sp, pic, 8, Point(w / 2, STARTING_POS_HOTSPOT_Y));
+				(sp, pic, 8, Point(pic->get_w() / 2, STARTING_POS_HOTSPOT_Y));
 		}
 	}
 

=== modified file 'src/graphic/graphic.cc'
--- src/graphic/graphic.cc	2012-12-16 17:06:00 +0000
+++ src/graphic/graphic.cc	2013-01-06 13:38:25 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2002-2004, 2006-2012 by the Widelands Development Team
+ * Copyright (C) 2002-2004, 2006-2013 by the Widelands Development Team
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -17,6 +17,7 @@
  *
  */
 
+#include "compile_diagnostics.h"
 #include "graphic.h"
 
 #include "build_info.h"
@@ -242,7 +243,9 @@
 		log("Graphics: OpenGL: Multitexture capabilities ");
 		log(m_caps.gl.multitexture ? "sufficient\n" : "insufficient, only basic terrain rendering possible\n");
 
+GCC_DIAG_OFF("-Wold-style-cast")
 		m_caps.gl.blendequation = GLEW_VERSION_1_4 || GLEW_ARB_imaging;
+GCC_DIAG_ON ("-Wold-style-cast")
 	}
 #endif
 
@@ -466,7 +469,6 @@
 
 	// First step: compute scaling factors
 	Rect srcrect = Rect(Point(0, 0), src->get_w(), src->get_h());
-	Rect destrect = Rect(Point(0, 0), w, h);
 
 	// Second step: get source material
 	SDL_Surface * srcsdl = 0;
@@ -483,7 +485,7 @@
 
 	// Third step: perform the zoom and placement
 	SDL_Surface * zoomed = zoomSurface
-		(srcsdl, double(destrect.w) / srcsdl->w, double(destrect.h) / srcsdl->h, 1);
+		(srcsdl, double(w) / srcsdl->w, double(h) / srcsdl->h, 1);
 	if (free_source)
 		SDL_FreeSurface(srcsdl);
 
@@ -492,39 +494,28 @@
 		SDL_Surface * placed = SDL_CreateRGBSurface
 			(SDL_SWSURFACE, w, h,
 			 fmt.BitsPerPixel, fmt.Rmask, fmt.Gmask, fmt.Bmask, fmt.Amask);
-		SDL_Rect srcrc = {0, 0, zoomed->w, zoomed->h};
-		SDL_Rect dstrc = {destrect.x, destrect.y};
+		SDL_Rect srcrc =
+			{0, 0,
+			 static_cast<Uint16>(zoomed->w), static_cast<Uint16>(zoomed->h)
+			};  // For some reason SDL_Surface and SDL_Rect express w,h in different types
+		SDL_Rect dstrc = {0, 0, 0, 0};
 		SDL_SetAlpha(zoomed, 0, 0);
-		SDL_BlitSurface(zoomed, &srcrc, placed, &dstrc);
+		SDL_BlitSurface(zoomed, &srcrc, placed, &dstrc); // Updates dstrc
 
 		Uint32 fillcolor = SDL_MapRGBA(zoomed->format, 0, 0, 0, 255);
 
-		if (destrect.x > 0) {
-			dstrc.x = 0;
-			dstrc.y = destrect.y;
-			dstrc.w = destrect.x;
-			dstrc.h = zoomed->h;
-			SDL_FillRect(placed, &dstrc, fillcolor);
-		}
-		if (destrect.x + zoomed->w < placed->w) {
-			dstrc.x = destrect.x + zoomed->w;
-			dstrc.y = destrect.y;
-			dstrc.w = placed->w - destrect.x - zoomed->w;
-			dstrc.h = zoomed->h;
-			SDL_FillRect(placed, &dstrc, fillcolor);
-		}
-		if (destrect.y > 0) {
-			dstrc.x = 0;
+		if (zoomed->w < placed->w) {
+			dstrc.x = zoomed->w;
 			dstrc.y = 0;
-			dstrc.w = placed->w;
-			dstrc.h = destrect.y;
+			dstrc.w = placed->w - zoomed->w;
+			dstrc.h = zoomed->h;
 			SDL_FillRect(placed, &dstrc, fillcolor);
 		}
-		if (destrect.y + zoomed->h < placed->h) {
+		if (zoomed->h < placed->h) {
 			dstrc.x = 0;
-			dstrc.y = destrect.y + zoomed->h;
+			dstrc.y = zoomed->h;
 			dstrc.w = placed->w;
-			dstrc.h = placed->h - destrect.y - zoomed->h;
+			dstrc.h = placed->h - zoomed->h;
 			SDL_FillRect(placed, &dstrc, fillcolor);
 		}
 
@@ -857,7 +848,7 @@
 /**
  * Load all animations that are registered with the AnimationManager
 */
-void Graphic::load_animations(UI::ProgressWindow & loader_ui) {
+void Graphic::load_animations() {
 	assert(m_animations.empty());
 
 	m_animations.reserve(g_anim.get_nranimations());

=== modified file 'src/graphic/graphic.h'
--- src/graphic/graphic.h	2012-12-31 11:21:15 +0000
+++ src/graphic/graphic.h	2013-01-06 13:38:25 +0000
@@ -132,7 +132,7 @@
 	void animate_maptextures(uint32_t time);
 	void reset_texture_animation_reminder();
 
-	void load_animations(UI::ProgressWindow & loader_ui);
+	void load_animations();
 	void ensure_animation_loaded(uint32_t anim);
 	AnimationGfx::Index nr_frames(uint32_t anim = 0);
 	uint32_t get_animation_frametime(uint32_t anim) const;
@@ -177,15 +177,15 @@
 	bool m_update_fullscreen;
 	/// stores which features the current renderer has
 	GraphicCaps m_caps;
+	Road_Textures * m_roadtextures;
+	const IPicture* m_edgetexture;
+	std::vector<Texture *> m_maptextures;
+	std::vector<AnimationGfx *> m_animations;
+
 	/// The class that gets images from disk.
 	boost::scoped_ptr<IImageLoader> img_loader_;
 	// The cache holding the images.
 	boost::scoped_ptr<ImageCache> img_cache_;
-
-	Road_Textures * m_roadtextures;
-	const IPicture* m_edgetexture;
-	std::vector<Texture *> m_maptextures;
-	std::vector<AnimationGfx *> m_animations;
 };
 
 extern Graphic * g_gr;

=== modified file 'src/graphic/render/gl_surface.cc'
--- src/graphic/render/gl_surface.cc	2012-12-15 18:40:59 +0000
+++ src/graphic/render/gl_surface.cc	2013-01-06 13:38:25 +0000
@@ -83,8 +83,6 @@
 void GLSurface::fill_rect(const Rect& rc, const RGBAColor clr) {
 	assert(rc.x >= 0);
 	assert(rc.y >= 0);
-	assert(rc.w >= 0);
-	assert(rc.h >= 0);
 	assert(g_opengl);
 	glDisable(GL_TEXTURE_2D);
 	glDisable(GL_BLEND);

=== modified file 'src/graphic/render/sdl_surface.cc'
--- src/graphic/render/sdl_surface.cc	2012-12-14 22:09:16 +0000
+++ src/graphic/render/sdl_surface.cc	2013-01-06 13:38:25 +0000
@@ -183,7 +183,10 @@
 	assert(rc.h >= 1);
 	const uint32_t color = clr.map(format());
 
-	SDL_Rect r = {rc.x, rc.y, rc.w, rc.h};
+	SDL_Rect r = {
+		static_cast<Sint16>(rc.x), static_cast<Sint16>(rc.y),
+		static_cast<Uint16>(rc.w), static_cast<Uint16>(rc.h)
+		};
 	SDL_FillRect(m_surface, &r, color);
 }
 
@@ -330,8 +333,14 @@
 	(const Point& dst, const IPicture* src, const Rect& srcrc, Composite cm)
 {
 	const SDLSurface* sdlsurf = static_cast<const SDLSurface*>(src);
-	SDL_Rect srcrect = {srcrc.x, srcrc.y, srcrc.w, srcrc.h};
-	SDL_Rect dstrect = {dst.x, dst.y, 0, 0};
+	SDL_Rect srcrect = {
+		static_cast<Sint16>(srcrc.x), static_cast<Sint16>(srcrc.y),
+		static_cast<Uint16>(srcrc.w), static_cast<Uint16>(srcrc.h)
+		};
+	SDL_Rect dstrect = {
+		static_cast<Sint16>(dst.x), static_cast<Sint16>(dst.y),
+		0, 0
+		};
 
 	bool alpha;
 	uint8_t alphaval;

=== modified file 'src/graphic/text/rt_render.cc'
--- src/graphic/text/rt_render.cc	2012-12-27 17:44:37 +0000
+++ src/graphic/text/rt_render.cc	2013-01-06 13:38:25 +0000
@@ -72,13 +72,17 @@
 };
 
 struct Reference {
-	Rect dim;
+	Rect dim; // w & h are uint32_t; x & y are int32_t
 	string ref;
 
+	// Why isn't Rect::contains() suitable?
+	// There is a small difference...
+	// Rect::contains() excludes the bottom and right edges.
+	// Reference::contains() includes the bottom and right edges
 	inline bool contains(int16_t x, int16_t y) const {
-		if (dim.x <= x and x <= dim.x + dim.w and dim.y <= y and y <= dim.y + dim.h)
-			return true;
-		return false;
+		return
+			dim.x <= x && x <= dim.x + static_cast<int32_t>(dim.w) &&
+			dim.y <= y && y <= dim.y + static_cast<int32_t>(dim.h);
 	}
 };
 
@@ -393,7 +397,7 @@
 	virtual uint32_t height() {return 0;}
 	virtual uint32_t width() {return INFINITE_WIDTH; }
 	virtual uint32_t hotspot_y() {return 0;}
-	virtual IBlitableSurface* render(IGraphic& gr) {
+	virtual IBlitableSurface* render(IGraphic& /* gr */) {
 		assert(false); // This should never be called
 	}
 	virtual bool is_non_mandatory_space() {return true;}
@@ -962,7 +966,8 @@
 
 	NodeStyle default_fs = {
 		"DejaVuSerif", 16,
-		RGBColor(0, 0, 0), IFont::DEFAULT, 0, HALIGN_LEFT, VALIGN_BOTTOM
+		RGBColor(0, 0, 0), IFont::DEFAULT, 0, HALIGN_LEFT, VALIGN_BOTTOM,
+		""
 	};
 
 	if (!width)

=== modified file 'src/graphic/text/sdl_ttf_font_impl.cc'
--- src/graphic/text/sdl_ttf_font_impl.cc	2012-12-16 19:08:53 +0000
+++ src/graphic/text/sdl_ttf_font_impl.cc	2013-01-06 13:38:25 +0000
@@ -96,8 +96,8 @@
 		// the only compatible way to do it using SDL 1.2. SDL 2.0 offers more
 		// functionality but is not yet released.
 		Uint8 sr, sg, sb, sa, dr, dg, db, da, outa, outr = 0, outg = 0, outb = 0;
-		for (uint32_t y = 0; y < tsurf->h; ++y) {
-			for (uint32_t x = 0; x < tsurf->w; ++x) {
+		for (int y = 0; y < tsurf->h; ++y) {
+			for (int x = 0; x < tsurf->w; ++x) {
 				size_t sidx = (y * tsurf->pitch + 4 * x) / 4;
 				size_t didx = ((y + SHADOW_OFFSET) * text_surface->pitch + (x + SHADOW_OFFSET) * 4) / 4;
 

=== modified file 'src/io/filesystem/disk_filesystem.cc'
--- src/io/filesystem/disk_filesystem.cc	2012-09-21 21:36:07 +0000
+++ src/io/filesystem/disk_filesystem.cc	2013-01-06 13:38:25 +0000
@@ -16,6 +16,7 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  *
  */
+#include "compile_diagnostics.h"
 
 #include "disk_filesystem.h"
 
@@ -425,12 +426,6 @@
 	return data;
 }
 
-#ifndef _MSC_VER
-/// \note The MAP_FAILED macro from glibc uses old-style cast. We can not fix
-/// this ourselves, so we temporarily turn the error into a warning. It is
-/// turned back into an error after this function.
-#pragma GCC diagnostic warning "-Wold-style-cast"
-#endif
 void * RealFSImpl::fastLoad
 	(const std::string & fname, size_t & length, bool & fast)
 {
@@ -455,22 +450,24 @@
 	data = mmap(0, length, PROT_READ, MAP_PRIVATE, file, 0);
 
 	//if mmap doesn't work for some strange reason try the old way
-	if (data == MAP_FAILED)
+GCC_DIAG_OFF("-Wold-style-cast")
+	if (data == MAP_FAILED) {
+GCC_DIAG_ON("-Wold-style-cast")
 		return Load(fname, length);
+	}
 
 	fast = true;
 
 	assert(data);
+GCC_DIAG_OFF("-Wold-style-cast")
 	assert(data != MAP_FAILED);
+GCC_DIAG_ON("-Wold-style-cast")
 
 	close(file);
 
 	return data;
 #endif
 }
-#ifndef _MSC_VER
-#pragma GCC diagnostic error "-Wold-style-cast"
-#endif
 
 /**
  * Write the given block of memory to the repository.

=== modified file 'src/logic/editor_game_base.cc'
--- src/logic/editor_game_base.cc	2012-09-21 21:36:07 +0000
+++ src/logic/editor_game_base.cc	2013-01-06 13:38:25 +0000
@@ -287,7 +287,7 @@
 
 	// TODO: load player graphics? (maybe)
 
-	g_gr->load_animations(loader_ui);
+	g_gr->load_animations();
 }
 
 /**

=== modified file 'src/minizip/unzip.cc'
--- src/minizip/unzip.cc	2012-04-07 15:12:06 +0000
+++ src/minizip/unzip.cc	2013-01-06 13:38:25 +0000
@@ -1,3 +1,7 @@
+/* Don't want to modify the minizip/zlib sources, so lets silence the warnings */
+#include "compile_diagnostics.h"
+GCC_DIAG_OFF("-Wold-style-cast")
+
 /*
 ================================================================================
 

=== modified file 'src/network/internet_gaming.cc'
--- src/network/internet_gaming.cc	2013-01-01 18:05:49 +0000
+++ src/network/internet_gaming.cc	2013-01-06 13:38:25 +0000
@@ -19,6 +19,7 @@
 
 #include "internet_gaming.h"
 
+#include "compile_diagnostics.h"
 #include "i18n.h"
 #include "io/filesystem/layered_filesystem.h"
 #include "internet_gaming_messages.h"
@@ -106,7 +107,9 @@
 	IPaddress peer;
 	if (hostent * const he = gethostbyname(m_meta.c_str())) {
 		peer.host = (reinterpret_cast<in_addr *>(he->h_addr_list[0]))->s_addr;
+GCC_DIAG_OFF("-Wold-style-cast")
 		peer.port = htons(m_port);
+GCC_DIAG_ON("-Wold-style-cast")
 	} else
 		throw warning
 			(_("Connection problem"), "%s", _("Widelands has not been able to connect to the metaserver."));

=== modified file 'src/network/nethost.cc'
--- src/network/nethost.cc	2013-01-05 18:43:00 +0000
+++ src/network/nethost.cc	2013-01-06 13:38:25 +0000
@@ -895,6 +895,8 @@
 			 d->settings.savegame ? Widelands::Game::Loaded : d->settings.scenario ?
 			 Widelands::Game::NewMPScenario : Widelands::Game::NewNonScenario);
 
+		delete tips;
+
 		// if this is an internet game, tell the metaserver that the game is done.
 		if (m_internet)
 			InternetGaming::ref().set_game_done();

=== modified file 'src/network/network_lan_promotion.cc'
--- src/network/network_lan_promotion.cc	2012-02-15 21:25:34 +0000
+++ src/network/network_lan_promotion.cc	2013-01-06 13:38:25 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2004-2010 by the Widelands Development Team
+ * Copyright (C) 2004-2010,2013 by the Widelands Development Team
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -20,6 +20,7 @@
 #include "network_lan_promotion.h"
 
 #include "build_info.h"
+#include "compile_diagnostics.h"
 #include "constants.h"
 
 #include "container_iterate.h"
@@ -48,6 +49,8 @@
 
 	for (int32_t i = 0; ifnames[i].if_index; ++i) {
 		strncpy (ifr.ifr_name, ifnames[i].if_name, IFNAMSIZ);
+
+GCC_DIAG_OFF("-Wold-style-cast")
 		if (ioctl(sock, SIOCGIFFLAGS, &ifr) < 0)
 			continue;
 
@@ -56,6 +59,7 @@
 
 		if (ioctl(sock, SIOCGIFBRDADDR, &ifr) < 0)
 			continue;
+GCC_DIAG_ON("-Wold-style-cast")
 
 		broadcast_addresses.push_back
 			(reinterpret_cast<sockaddr_in *>(&ifr.ifr_broadaddr)
@@ -75,39 +79,34 @@
 	closesocket (sock);
 }
 
-/// \note The INADDR_ANY macro from glibc uses old-style cast. We can not fix
-/// this ourselves, so we temporarily turn the error into a warning. It is
-/// turned back into an error after this function.
-#pragma GCC diagnostic warning "-Wold-style-cast"
 void LAN_Base::bind (uint16_t port)
 {
 	sockaddr_in addr;
+
+GCC_DIAG_OFF("-Wold-style-cast")
 	addr.sin_family      = AF_INET;
 	addr.sin_addr.s_addr = INADDR_ANY;
 	addr.sin_port        = htons(port);
+GCC_DIAG_ON("-Wold-style-cast")
 
 	::bind (sock, reinterpret_cast<sockaddr *>(&addr), sizeof(addr));
 }
-#pragma GCC diagnostic error "-Wold-style-cast"
 
-/// \note The FD_SET macro from glibc uses old-style cast. We can not fix this
-/// ourselves, so we temporarily turn the error into a warning. It is turned
-/// back into an error after this function.
-#pragma GCC diagnostic warning "-Wold-style-cast"
 bool LAN_Base::avail ()
 {
 	fd_set fds;
 	timeval tv;
 
+GCC_DIAG_OFF("-Wold-style-cast")
 	FD_ZERO(&fds);
 	FD_SET(sock, &fds);
+GCC_DIAG_ON("-Wold-style-cast")
 
 	tv.tv_sec  = 0;
 	tv.tv_usec = 0;
 
 	return select(sock + 1, &fds, 0, 0, &tv) == 1;
 }
-#pragma GCC diagnostic error "-Wold-style-cast"
 
 ssize_t LAN_Base::recv
 	(void * const buf, size_t const len, sockaddr_in * const addr)
@@ -135,9 +134,6 @@
 		 sizeof(sockaddr_in));
 }
 
-//  FIXME Document why this pragma is needed and what is done about it (see how
-//  FIXME other uses of this pragmas are documented).
-#pragma GCC diagnostic warning "-Wold-style-cast"
 void LAN_Base::broadcast
 	(void const * const buf, size_t const len, uint16_t const port)
 {
@@ -145,7 +141,9 @@
 		sockaddr_in addr;
 		addr.sin_family      = AF_INET;
 		addr.sin_addr.s_addr = *i.current;
+GCC_DIAG_OFF("-Wold-style-cast")
 		addr.sin_port        = htons(port);
+GCC_DIAG_ON("-Wold-style-cast")
 
 		sendto
 			(sock,
@@ -156,7 +154,6 @@
 			 sizeof(addr));
 	}
 }
-#pragma GCC diagnostic error "-Wold-style-cast"
 
 /*** class LAN_Game_Promoter ***/
 
@@ -240,9 +237,6 @@
 }
 
 
-//  FIXME Document why this pragma is needed and what is done about it (see how
-//  FIXME other uses of this pragmas are documented).
-#pragma GCC diagnostic warning "-Wold-style-cast"
 void LAN_Game_Finder::run ()
 {
 	while (avail()) {
@@ -267,8 +261,10 @@
 		for (wl_const_range<std::list<Net_Open_Game *> > i(opengames);; ++i)
 			if (i.empty()) {
 				opengames.push_back (new Net_Open_Game);
+GCC_DIAG_OFF("-Wold-style-cast")
 				opengames.back()->address = addr.sin_addr.s_addr;
 				opengames.back()->port    = htons(WIDELANDS_PORT);
+GCC_DIAG_ON("-Wold-style-cast")
 				opengames.back()->info    = info;
 				callback (GameOpened, opengames.back(), userdata);
 				break;
@@ -280,7 +276,6 @@
 			}
 	}
 }
-#pragma GCC diagnostic error "-Wold-style-cast"
 
 void LAN_Game_Finder::set_callback
 	(void (* const cb)(int32_t, Net_Open_Game const *, void *), void * const ud)

=== modified file 'src/s2map.cc'
--- src/s2map.cc	2012-09-21 21:36:07 +0000
+++ src/s2map.cc	2013-01-06 13:38:25 +0000
@@ -31,14 +31,12 @@
 #include "wexception.h"
 
 #include <iostream>
+#include <iomanip>
 
 using std::cerr;
+using std::endl;
 using std::ios;
-using std::endl;
-
-// TEMP
-#define hex ios::hex
-#define dec ios::dec
+using std::setiosflags;
 
 // this is a detail of S2 maps
 #define CRITTER_PER_DEFINITION   1
@@ -328,9 +326,11 @@
 				default:
 					c = 7;
 					cerr
-						<< "ERROR: Unknown texture1: " << hex << c << dec << " ("
+						<< "ERROR: Unknown texture1: "
+						<< setiosflags(ios::hex) << c
+						<< setiosflags(ios::dec) << " ("
 						<< x << "," << y << ") (defaults to water!)" << endl;
-					break;
+						break;
 				}
 				f->set_terrain_d(c);
 			}
@@ -380,7 +380,9 @@
 				default:
 					c = 7;
 					cerr
-						<< "ERROR: Unknown texture1: " << hex << c << dec << " ("
+						<< "ERROR: Unknown texture1: "
+						<< setiosflags(ios::hex) << c
+						<< setiosflags(ios::dec) << " ("
 						<< x << "," << y << ") (defaults to water!)" << endl;
 					break;
 				}

=== modified file 'src/scripting/pluto.cc'
--- src/scripting/pluto.cc	2012-09-21 21:36:07 +0000
+++ src/scripting/pluto.cc	2013-01-06 13:38:25 +0000
@@ -27,6 +27,11 @@
 
 #include "pluto.h"
 
+// Widelands: silence warnings about unused variables, usually because they
+//are only used in conditional asserts
+#include "compile_diagnostics.h"
+GCC_DIAG_OFF("-Wunused-variable")
+
 
 // Forward declarated from lua_impl.h. So we do not need to include it
 int luna_restore_object(lua_State * L);

=== modified file 'src/ui_basic/listselect.cc'
--- src/ui_basic/listselect.cc	2012-12-16 14:29:46 +0000
+++ src/ui_basic/listselect.cc	2013-01-06 13:38:25 +0000
@@ -411,7 +411,6 @@
 
 		// Now draw pictures
 		if (er.pic) {
-			uint32_t w = er.pic->get_w();
 			uint32_t h = er.pic->get_h();
 			dst.blit(Point(1, y + (get_lineheight() - h) / 2), er.pic);
 		}

=== modified file 'src/ui_basic/panel.cc'
--- src/ui_basic/panel.cc	2013-01-04 09:59:13 +0000
+++ src/ui_basic/panel.cc	2013-01-06 13:38:25 +0000
@@ -305,8 +305,6 @@
 	if (_desired_w == w && _desired_h == h)
 		return;
 
-	assert(w >= 0);
-	assert(h >= 0);
 	assert(w < 3000);
 	assert(h < 3000);
 

=== modified file 'src/ui_fsmenu/base.cc'
--- src/ui_fsmenu/base.cc	2012-12-15 15:18:49 +0000
+++ src/ui_fsmenu/base.cc	2013-01-06 13:38:25 +0000
@@ -54,8 +54,6 @@
 	: UI::Panel(0, 0, 0, gr_x(), gr_y()),
 	d(new Data)
 {
-	Section & s = g_options.pull_section("global");
-
 	// Load background graphics
 	char buffer[256];
 	snprintf(buffer, sizeof(buffer), "pics/%s", bgpic);

=== modified file 'src/ui_fsmenu/internet_lobby.cc'
--- src/ui_fsmenu/internet_lobby.cc	2012-12-13 10:41:22 +0000
+++ src/ui_fsmenu/internet_lobby.cc	2013-01-06 13:38:25 +0000
@@ -21,6 +21,7 @@
 
 #include <boost/bind.hpp>
 
+#include "compile_diagnostics.h"
 #include "constants.h"
 #include "graphic/graphic.h"
 #include "i18n.h"
@@ -399,7 +400,9 @@
 		IPaddress peer;
 		if (hostent * const he = gethostbyname(ip.c_str())) {
 			peer.host = (reinterpret_cast<in_addr *>(he->h_addr_list[0]))->s_addr;
+GCC_DIAG_OFF("-Wold-style-cast")
 			peer.port = htons(WIDELANDS_PORT);
+GCC_DIAG_ON("-Wold-style-cast")
 		} else {
 			// Actually the game is not done, but that way we are again listed as in the lobby
 			InternetGaming::ref().set_game_done();

=== modified file 'src/ui_fsmenu/netsetup_lan.cc'
--- src/ui_fsmenu/netsetup_lan.cc	2012-12-13 10:41:22 +0000
+++ src/ui_fsmenu/netsetup_lan.cc	2013-01-06 13:38:25 +0000
@@ -18,6 +18,8 @@
  */
 
 #include "netsetup_lan.h"
+
+#include "compile_diagnostics.h"
 #include "constants.h"
 #include "graphic/graphic.h"
 #include "i18n.h"
@@ -156,7 +158,9 @@
 
 	if (hostent * const he = gethostbyname(host.c_str())) {
 		addr = (reinterpret_cast<in_addr *>(he->h_addr_list[0]))->s_addr;
+GCC_DIAG_OFF("-Wold-style-cast")
 		port = htons(WIDELANDS_PORT);
+GCC_DIAG_ON("-Wold-style-cast")
 		return true;
 	} else
 		return false;

=== modified file 'src/wui/waresdisplay.cc'
--- src/wui/waresdisplay.cc	2012-12-27 17:44:37 +0000
+++ src/wui/waresdisplay.cc	2013-01-06 13:38:25 +0000
@@ -246,7 +246,6 @@
 			 ware_selected(id) ?  "pics/ware_list_bg_selected.png"
 			                   :  "pics/ware_list_bg.png");
 	uint32_t w = bgpic->get_w();
-	uint32_t h = bgpic->get_h();
 
 	dst.blit(p, bgpic);
 
@@ -410,7 +409,6 @@
 				//  draw a background
 				const IPicture* pic = g_gr->imgcache().load (PicMod_Game, "pics/ware_list_bg.png");
 				uint32_t w = pic->get_w();
-				uint32_t h = pic->get_h();
 
 				dst.blit(p, pic);
 

=== modified file 'src/wui/waresqueuedisplay.cc'
--- src/wui/waresqueuedisplay.cc	2012-12-13 10:41:22 +0000
+++ src/wui/waresqueuedisplay.cc	2013-01-06 13:38:25 +0000
@@ -64,7 +64,6 @@
 	m_icon_grey = g_gr->create_grayed_out_pic(m_icon);
 	m_icon_grey = g_gr->create_changed_luminosity_pic(m_icon_grey, 0.65);
 
-	uint32_t pw = m_max_fill_indicator->get_w();
 	uint32_t ph = m_max_fill_indicator->get_h();
 
 	uint32_t priority_button_height = show_only ? 0 : 3 * PriorityButtonSize;