widelands-dev team mailing list archive

[SirVer <SirVer@xxxxxx>]

SirVer has proposed merging lp:~widelands-dev/widelands/avoid_wraparound into lp:widelands.

Commit message:
Switched overzealous uint32_t -> int to avoid underflow errors on minus arithmetic.

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1522582 in widelands: "vertical stripes if port window is on the top of screen"
  https://bugs.launchpad.net/widelands/+bug/1522582

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/avoid_wraparound/+merge/284067

Fixes stripes of the port window.

The problem is that we do unsigned d = (unsized(0) - unsized(50)) / 2, which is is a huge number. 

later we compare

2 *(d + ~50)  < 1000 

which is true, because we wrap around the other way again. 

The solution is to never use unsigned integers unless absolutely needed. I changed a lot of pixel size to be int instead of uint32_t. 
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/avoid_wraparound into lp:widelands.

=== modified file 'src/graphic/rendertarget.cc'
--- src/graphic/rendertarget.cc	2016-01-23 14:54:44 +0000
+++ src/graphic/rendertarget.cc	2016-01-27 08:11:43 +0000
@@ -329,10 +329,10 @@
 	Rect destination_rect(dst.x - animation.hotspot().x + source_rect.x,
 	                      dst.y - animation.hotspot().y + source_rect.y, source_rect.w,
 	                      source_rect.h);
-
 	Rect srcrc(source_rect);
-	if (to_surface_geometry(&destination_rect, &srcrc))
+	if (to_surface_geometry(&destination_rect, &srcrc)) {
 		animation.blit(time, destination_rect.origin(), srcrc, player_color, m_surface);
+	}
 
 	// Look if there is a sound effect registered for this frame and trigger the
 	// effect (see SoundHandler::stereo_position).

=== modified file 'src/logic/editor_game_base.cc'
--- src/logic/editor_game_base.cc	2016-01-17 09:54:31 +0000
+++ src/logic/editor_game_base.cc	2016-01-27 08:11:43 +0000
@@ -634,9 +634,8 @@
 	assert     (player_area.x < map().get_width());
 	assert(0 <= player_area.y);
 	assert     (player_area.y < map().get_height());
-	const Field & first_field = map()[0];
-	assert(&first_field <= player_area.field);
-	assert(player_area.field < &first_field + map().max_index());
+	assert(&map()[0] <= player_area.field);
+	assert(player_area.field < &map()[0] + map().max_index());
 	assert(0 < player_area.player_number);
 	assert    (player_area.player_number <= map().get_nrplayers());
 	MapRegion<Area<FCoords> > mr(map(), player_area);

=== modified file 'src/logic/map_objects/tribes/ship.h'
--- src/logic/map_objects/tribes/ship.h	2016-01-18 19:08:41 +0000
+++ src/logic/map_objects/tribes/ship.h	2016-01-27 08:11:43 +0000
@@ -280,7 +280,6 @@
 		uint32_t m_destination;
 		uint8_t  m_ship_state;
 		std::string m_shipname;
-		uint32_t m_shipname_index;
 		std::unique_ptr<Expedition> m_expedition;
 		std::vector<ShippingItem::Loader> m_items;
 	};

=== modified file 'src/ui_basic/box.cc'
--- src/ui_basic/box.cc	2016-01-17 08:29:59 +0000
+++ src/ui_basic/box.cc	2016-01-27 08:11:43 +0000
@@ -88,12 +88,12 @@
  */
 void Box::update_desired_size()
 {
-	uint32_t totaldepth = 0;
-	uint32_t maxbreadth = m_mindesiredbreadth;
+	int totaldepth = 0;
+	int maxbreadth = m_mindesiredbreadth;
 
 	for (uint32_t idx = 0; idx < m_items.size(); ++idx) {
-		uint32_t depth, breadth;
-		get_item_desired_size(idx, depth, breadth);
+		int depth, breadth;
+		get_item_desired_size(idx, &depth, &breadth);
 
 		totaldepth += depth;
 		if (breadth > maxbreadth)
@@ -131,11 +131,11 @@
 void Box::layout()
 {
 	// First pass: compute the depth and adjust whether we have a scrollbar
-	uint32_t totaldepth = 0;
+	int totaldepth = 0;
 
-	for (uint32_t idx = 0; idx < m_items.size(); ++idx) {
-		uint32_t depth, tmp;
-		get_item_desired_size(idx, depth, tmp);
+	for (size_t idx = 0; idx < m_items.size(); ++idx) {
+		int depth, tmp;
+		get_item_desired_size(idx, &depth, &tmp);
 
 		totaldepth += depth;
 	}
@@ -223,8 +223,8 @@
 		totalbreadth -= Scrollbar::Size;
 
 	for (uint32_t idx = 0; idx < m_items.size(); ++idx) {
-		uint32_t depth, breadth;
-		get_item_size(idx, depth, breadth);
+		int depth, breadth;
+		get_item_size(idx, &depth, &breadth);
 
 		if (m_items[idx].type == Item::ItemPanel) {
 			set_item_size
@@ -317,7 +317,7 @@
  * to the orientation axis.
 */
 void Box::get_item_desired_size
-	(uint32_t const idx, uint32_t & depth, uint32_t & breadth)
+	(uint32_t const idx, int* depth, int* breadth)
 {
 	assert(idx < m_items.size());
 
@@ -333,7 +333,7 @@
 		break;
 
 	case Item::ItemSpace:
-		depth   = it.u.space;
+		*depth = it.u.space;
 		breadth = 0;
 		break;
 	}
@@ -344,7 +344,7 @@
  * for expanding items, at least for now.
  */
 void Box::get_item_size
-	(uint32_t const idx, uint32_t & depth, uint32_t & breadth)
+	(uint32_t const idx, int* depth, int* breadth)
 {
 	assert(idx < m_items.size());
 
@@ -357,7 +357,7 @@
 /**
  * Set the given items actual size.
  */
-void Box::set_item_size(uint32_t idx, uint32_t depth, uint32_t breadth)
+void Box::set_item_size(uint32_t idx, int depth, int breadth)
 {
 	assert(idx < m_items.size());
 

=== modified file 'src/ui_basic/box.h'
--- src/ui_basic/box.h	2014-07-26 10:43:23 +0000
+++ src/ui_basic/box.h	2016-01-27 08:11:43 +0000
@@ -71,15 +71,15 @@
 	void update_desired_size() override;
 
 private:
-	void get_item_desired_size(uint32_t idx, uint32_t & depth, uint32_t & breadth);
-	void get_item_size(uint32_t idx, uint32_t & depth, uint32_t & breadth);
-	void set_item_size(uint32_t idx, uint32_t depth, uint32_t breadth);
+	void get_item_desired_size(uint32_t idx, int* depth, int* breadth);
+	void get_item_size(uint32_t idx, int* depth, int* breadth);
+	void set_item_size(uint32_t idx, int depth, int breadth);
 	void set_item_pos(uint32_t idx, int32_t pos);
 	void scrollbar_moved(int32_t);
 	void update_positions();
 
 	//don't resize beyond this size
-	uint32_t m_max_x, m_max_y;
+	int m_max_x, m_max_y;
 
 private:
 	struct Item {
@@ -93,14 +93,14 @@
 		union {
 			struct {
 				Panel * panel;
-				uint32_t align;
+				int align;
 				bool fullsize;
 			} panel;
-			uint32_t space;
+			int space;
 		} u;
 
 		bool fillspace;
-		uint32_t assigned_var_depth;
+		int assigned_var_depth;
 	};
 
 	bool m_scrolling;

=== modified file 'src/ui_basic/panel.cc'
--- src/ui_basic/panel.cc	2015-11-28 11:04:12 +0000
+++ src/ui_basic/panel.cc	2016-01-27 08:11:43 +0000
@@ -50,7 +50,7 @@
  */
 Panel::Panel
 	(Panel * const nparent,
-	 const int32_t nx, const int32_t ny, const uint32_t nw, const uint32_t nh,
+	 const int nx, const int ny, const int nw, const int nh,
 	 const std::string & tooltip_text)
 	:
 	_parent(nparent), _fchild(nullptr), _lchild(nullptr), _mousein(nullptr), _focus(nullptr),
@@ -154,7 +154,7 @@
 
 	uint32_t minTime;
 	{
-		int32_t maxfps = g_options.pull_section("global").get_int("maxfps", 25);
+		int maxfps = g_options.pull_section("global").get_int("maxfps", 25);
 		if (maxfps < 5)
 			maxfps = 5;
 		minTime = 1000 / maxfps;
@@ -237,13 +237,13 @@
  * \note NEVER override this function. If you feel the urge to override this
  * function, you probably want to override \ref layout.
  */
-void Panel::set_size(const uint32_t nw, const uint32_t nh)
+void Panel::set_size(const int nw, const int nh)
 {
 	if (nw == _w && nh == _h)
 		return;
 
-	uint32_t const upw = std::min(nw, _w);
-	uint32_t const uph = std::min(nh, _h);
+	int const upw = std::min(nw, _w);
+	int const uph = std::min(nh, _h);
 	_w = nw;
 	_h = nh;
 
@@ -269,10 +269,10 @@
  * Set \p w and \p h to the desired
  * width and height of this panel, respectively.
  */
-void Panel::get_desired_size(uint32_t & w, uint32_t & h) const
+void Panel::get_desired_size(int* w, int* h) const
 {
-	w = _desired_w;
-	h = _desired_h;
+	*w = _desired_w;
+	*h = _desired_h;
 }
 
 /**
@@ -285,7 +285,7 @@
  *
  * \note NEVER override this function
  */
-void Panel::set_desired_size(uint32_t w, uint32_t h)
+void Panel::set_desired_size(int w, int h)
 {
 	if (_desired_w == w && _desired_h == h)
 		return;
@@ -367,7 +367,7 @@
 /**
  * Set the size of the inner area (total area minus border)
  */
-void Panel::set_inner_size(uint32_t const nw, uint32_t const nh)
+void Panel::set_inner_size(int const nw, int const nh)
 {
 	set_size(nw + _lborder + _rborder, nh + _tborder + _bborder);
 }
@@ -377,7 +377,7 @@
  * Note that since position and total size aren't changed, so that the size
  * and position of the inner area will change.
  */
-void Panel::set_border(uint32_t l, uint32_t r, uint32_t t, uint32_t b)
+void Panel::set_border(int l, int r, int t, int b)
 {
 	_lborder = l;
 	_rborder = r;
@@ -451,12 +451,12 @@
 /**
  * Mark a part of a panel for updating.
  */
-void Panel::update(int32_t x, int32_t y, int32_t w, int32_t h)
+void Panel::update(int x, int y, int w, int h)
 {
 	if
-		(x >= static_cast<int32_t>(_w) || x + w <= 0
+		(x >= _w || x + w <= 0
 		 ||
-		 y >= static_cast<int32_t>(_h) || y + h <= 0)
+		 y >= _h || y + h <= 0)
 		return;
 
 	if (_parent) {

=== modified file 'src/ui_basic/panel.h'
--- src/ui_basic/panel.h	2016-01-15 21:46:03 +0000
+++ src/ui_basic/panel.h	2016-01-27 08:11:43 +0000
@@ -79,7 +79,7 @@
 	Panel
 		(Panel * const nparent,
 		 int32_t  const nx, int32_t  const ny,
-		 uint32_t const nw, uint32_t const nh,
+		 int const nw, int const nh,
 		 const std::string& tooltip_text = std::string());
 	virtual ~Panel();
 
@@ -114,8 +114,8 @@
 	virtual void end();
 
 	// Geometry
-	void set_size(uint32_t nw, uint32_t nh);
-	void set_desired_size(uint32_t w, uint32_t h);
+	void set_size(int nw, int nh);
+	void set_desired_size(int w, int h);
 	void set_pos(Point);
 	virtual void move_inside_parent();
 	virtual void layout();
@@ -123,7 +123,7 @@
 	void set_layout_toplevel(bool ltl);
 	bool get_layout_toplevel() const;
 
-	void get_desired_size(uint32_t & w, uint32_t & h) const;
+	void get_desired_size(int* w, int* h) const;
 
 	int32_t get_x() const {return _x;}
 	int32_t get_y() const {return _y;}
@@ -151,19 +151,19 @@
 		return _flags & pf_dock_windows_to_edges;
 	}
 	void set_dock_windows_to_edges(const bool on = true);
-	void set_inner_size(uint32_t nw, uint32_t nh);
-	void set_border(uint32_t l, uint32_t r, uint32_t t, uint32_t b);
-
-	uint32_t get_lborder() const {return _lborder;}
-	uint32_t get_rborder() const {return _rborder;}
-	uint32_t get_tborder() const {return _tborder;}
-	uint32_t get_bborder() const {return _bborder;}
-
-	uint32_t get_inner_w() const {
+	void set_inner_size(int nw, int nh);
+	void set_border(int l, int r, int t, int b);
+
+	int get_lborder() const {return _lborder;}
+	int get_rborder() const {return _rborder;}
+	int get_tborder() const {return _tborder;}
+	int get_bborder() const {return _bborder;}
+
+	int get_inner_w() const {
 		assert(_lborder + _rborder <= _w);
 		return _w - (_lborder + _rborder);
 	}
-	uint32_t get_inner_h() const {
+	int get_inner_h() const {
 		assert(_tborder + _bborder <= _h);
 		return _h - (_tborder + _bborder);
 	}
@@ -321,11 +321,11 @@
 	 */
 	/*@{*/
 	int32_t _x, _y;
-	uint32_t _w, _h;
+	int _w, _h;
 	/*@}*/
-	uint32_t _lborder, _rborder, _tborder, _bborder;
+	int _lborder, _rborder, _tborder, _bborder;
 	uint8_t _border_snap_distance, _panel_snap_distance;
-	uint32_t _desired_w, _desired_h;
+	int _desired_w, _desired_h;
 
 	bool _running;
 	int _retcode;
@@ -359,7 +359,7 @@
 	NamedPanel
 		(Panel * const nparent, const std::string & name,
 		 int32_t  const nx, int32_t  const ny,
-		 uint32_t const nw, uint32_t const nh,
+		 int const nw, int const nh,
 		 const std::string & tooltip_text = std::string())
 		: Panel(nparent, nx, ny, nw, nh, tooltip_text), m_name(name)
 	{

=== modified file 'src/ui_basic/table.cc'
--- src/ui_basic/table.cc	2016-01-26 09:10:52 +0000
+++ src/ui_basic/table.cc	2016-01-27 08:11:43 +0000
@@ -248,9 +248,9 @@
 	if (entries == 0) {
 		entries = size();
 	}
-	uint32_t tablewidth;
-	uint32_t tableheight;
-	get_desired_size(tablewidth, tableheight);
+	int tablewidth;
+	int tableheight;
+	get_desired_size(&tablewidth, &tableheight);
 	tableheight = m_headerheight + 2 + get_lineheight() * entries;
 	set_desired_size(tablewidth, tableheight);
 }

=== modified file 'src/ui_basic/tabpanel.cc'
--- src/ui_basic/tabpanel.cc	2016-01-01 19:04:45 +0000
+++ src/ui_basic/tabpanel.cc	2016-01-27 08:11:43 +0000
@@ -139,19 +139,16 @@
  */
 void TabPanel::update_desired_size()
 {
-	uint32_t w;
-	uint32_t h;
-
 	// size of button row
-	w = kTabPanelButtonHeight * tabs_.size();
-	h = kTabPanelButtonHeight + kTabPanelSeparatorHeight;
+	int w = kTabPanelButtonHeight * tabs_.size();
+	int h = kTabPanelButtonHeight + kTabPanelSeparatorHeight;
 
 	// size of contents
 	if (active_ < tabs_.size()) {
 		Panel * const panel = tabs_[active_]->panel;
-		uint32_t panelw, panelh;
+		int panelw, panelh;
 
-		panel->get_desired_size(panelw, panelh);
+		panel->get_desired_size(&panelw, &panelh);
 		// TODO(unknown):  the panel might be bigger -> add a scrollbar in that case
 		//panel->set_size(panelw, panelh);
 

=== modified file 'src/ui_basic/textarea.cc'
--- src/ui_basic/textarea.cc	2015-12-11 19:03:45 +0000
+++ src/ui_basic/textarea.cc	2016-01-27 08:11:43 +0000
@@ -206,8 +206,8 @@
 	int32_t y = get_y();
 
 	update_desired_size();
-	uint32_t w, h;
-	get_desired_size(w, h);
+	int w, h;
+	get_desired_size(&w, &h);
 
 	if      (m_align & Align_HCenter)
 		x -= w >> 1;

=== modified file 'src/ui_basic/window.cc'
--- src/ui_basic/window.cc	2015-08-06 17:14:34 +0000
+++ src/ui_basic/window.cc	2016-01-27 08:11:43 +0000
@@ -133,8 +133,8 @@
 void Window::update_desired_size()
 {
 	if (m_center_panel) {
-		uint32_t innerw, innerh;
-		m_center_panel->get_desired_size(innerw, innerh);
+		int innerw, innerh;
+		m_center_panel->get_desired_size(&innerw, &innerh);
 		set_desired_size
 			(innerw + get_lborder() + get_rborder(),
 			 innerh + get_tborder() + get_bborder());
@@ -201,22 +201,20 @@
 	if (Panel * const parent = get_parent()) {
 		int32_t px = get_x();
 		int32_t py = get_y();
-		if
-			((parent->get_inner_w() < static_cast<uint32_t>(get_w())) &&
-			(px + get_w() <= static_cast<int32_t>(parent->get_inner_w()) || px >= 0))
-			px = (static_cast<int32_t>(parent->get_inner_w()) - get_w()) / 2;
-		if
-			((parent->get_inner_h() < static_cast<uint32_t>(get_h())) &&
-			(py + get_h() < static_cast<int32_t>(parent->get_inner_h()) || py > 0))
+		if ((parent->get_inner_w() < get_w()) && (px + get_w() <= parent->get_inner_w() || px >= 0))
+			px = (parent->get_inner_w() - get_w()) / 2;
+		if
+			((parent->get_inner_h() < get_h()) &&
+			(py + get_h() < parent->get_inner_h() || py > 0))
 				py = 0;
 
-		if (parent->get_inner_w() >= static_cast<uint32_t>(get_w())) {
+		if (parent->get_inner_w() >= get_w()) {
 			if (px < 0) {
 				px = 0;
 				if (parent->get_dock_windows_to_edges() && !_docked_left)
 					_docked_left =  true;
-			} else if (px + static_cast<uint32_t>(get_w()) >= parent->get_inner_w()) {
-				px = static_cast<int32_t>(parent->get_inner_w()) - get_w();
+			} else if (px + get_w() >= parent->get_inner_w()) {
+				px = parent->get_inner_w() - get_w();
 				if (parent->get_dock_windows_to_edges() && !_docked_right)
 					_docked_right = true;
 			}
@@ -225,11 +223,11 @@
 			else if (_docked_right)
 				px += VT_B_PIXMAP_THICKNESS;
 		}
-		if (parent->get_inner_h() >= static_cast<uint32_t>(get_h())) {
+		if (parent->get_inner_h() >= get_h()) {
 			if (py < 0)
 				py = 0;
-			else if (py + static_cast<uint32_t>(get_h()) > parent->get_inner_h()) {
-				py = static_cast<int32_t>(parent->get_inner_h()) - get_h();
+			else if (py + get_h() > parent->get_inner_h()) {
+				py = parent->get_inner_h() - get_h();
 				if
 					(!_is_minimal
 					&&

=== modified file 'src/wui/ware_statistics_menu.cc'
--- src/wui/ware_statistics_menu.cc	2016-01-24 20:11:53 +0000
+++ src/wui/ware_statistics_menu.cc	2016-01-27 08:11:43 +0000
@@ -115,8 +115,8 @@
 		 AbstractWaresDisplay(parent, x, y, tribe, Widelands::wwWARE, true, callback_function),
 		 color_map_(color_map)
 	{
-		uint32_t w, h;
-		get_desired_size(w, h);
+		int w, h;
+		get_desired_size(&w, &h);
 		set_size(w, h);
 	}
 protected: