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

[Jason Dorje Short <vze2zq63@xxxxxxxxxxx>]

Raahul Kumar wrote:
> 
> --- 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>

The bad code was introduced more recently than this.  From cvs annotate:

1.1          (andi     04-Sep-01):   assert(normalize_map_pos(&x, &y));
1.1          (andi     04-Sep-01):        
assert(normalize_map_pos(&dest_x, &dest_y));
1.1          (andi     04-Sep-01):        
assert(normalize_map_pos(&dest_x, &dest_y));

not good...but in this case the win32 client doesn't work anyway
(AFAIK), so IMO it's more important to have speedy development than to
make sure everything is correct.

jason