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

[Raahul Kumar <raahul_da_man@xxxxxxxxx>]

--- jdorje@xxxxxxxxxxxxxxxxxxxxx wrote:
> 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.
> 

Good save. I though this was fixed much earlier. 

Sat Jul 28 16:39:11 2001  Thue Janus Kristensen <thue@xxxxxxx>:
 
         * client/goto.c, client/gui-gtk/mapview.c,
         client/gui-mui/graphics.c, client/gui-mui/mapclass.c,
         client/gui-xaw/mapview.c, server/citytools.c, server/unittools.c:
 
         Don't run code inside an assert when we depend on the sideeffects.
         (ie, mostly fix "assert(normalize_map_pos(&x, &y))").
 
         Fixes (PR#864) reported by Gaute Strokkenes <gs234@xxxxxxxxx>


> 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);
>        }
>      }
> 


__________________________________________________
Do You Yahoo!?
Make a great connection at Yahoo! Personals.
http://personals.yahoo.com