[Freeciv-Dev] assert(normalize_map_pos(...)) is bad! (PR#1004)

[jdorje@xxxxxxxxxxxxxxxxxxxxx]

Some of the new (I think) gui-win32 code uses constructs like:

  assert(normalize_map_pos(&x, &y));

this is very bad!  normalize_map_pos has side effects, and putting it
inside an assert will mean (x, y) doesn't get normalized when not in
debugging mode; you'll get a near-certain segfault.

Instead you should use either

  assert(is_real_tile(x, y));
  normalize_map_pos(&x, &y);

or

  int is_real = normalize_map_pos(&x, &y);
  assert(is_real);

I think Raimar may be trying to make the latter the standard (although
if so this should be announced and a macro should be created).

The attached patch hopes to fix this (of course, I can't compile or test
it).

jason

? rc
? old
? topology
Index: client/gui-win32/mapview.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-win32/mapview.c,v
retrieving revision 1.5
diff -u -r1.5 mapview.c
--- client/gui-win32/mapview.c  2001/09/23 16:09:37     1.5
+++ client/gui-win32/mapview.c  2001/10/11 21:51:37
@@ -1650,6 +1650,7 @@
   int count, i = 0;
   int fog;
   int solid_bg;
+  int is_real;
 
   if (!width || !(height || height_unit))
     return;
@@ -1662,7 +1663,8 @@
                               offset_x, offset_y, width, height);
     return;
   }
-  assert(normalize_map_pos(&x, &y));
+  is_real = normalize_map_pos(&x, &y);
+  assert(is_real);
   fog = tile_is_known(x, y) == TILE_KNOWN_FOGGED && draw_fog_of_war;
   pcity = map_get_city(x, y);
   punit = get_drawable_unit(x, y, citymode);
@@ -1970,12 +1972,14 @@
                          on the adjacent tile when drawing in this direction. 
*/
         dest_x = src_x + 1;
         dest_y = src_y;
-        assert(normalize_map_pos(&dest_x, &dest_y));
+        is_real = normalize_map_pos(&dest_x, &dest_y);
+       assert(is_real);
         refresh_tile_mapcanvas(dest_x, dest_y, 1);
       } else if (dir == DIR8_SOUTHWEST) { /* the same */
         dest_x = src_x;
         dest_y = src_y + 1;
-        assert(normalize_map_pos(&dest_x, &dest_y));
+        is_real = normalize_map_pos(&dest_x, &dest_y);
+       assert(is_real);
         refresh_tile_mapcanvas(dest_x, dest_y, 1);
       }
     }