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

[Andreas Kemnade <akemnade@xxxxxxxxxxx>]

Jason Dorje Short writes:
 > 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));

That not the full truth.
AFAIK the code was available for download since Apr 2001.
But it was not in cvs yet. So Thue could not correct it. And I have
forgotten to look for it. I have never defined NDEBUG so I did not
notice the bug.

I'll commit the patch soon.
 > 
 > 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.

Summary for the win32 client:
- most things work.
- dialogs missing: diplomacy dialog, worklists
- some stuff on the left side of the main window is missing (timout
  label, unit list under it.
- small map drawing issues (dither_tile is commented out, smooth
  movement and hitpoint decrease)
- at least gui-stuff.h needs some more comments
- compilable with cygwin tools or mingw32 cross compiler (from linux)
- runs almost perfect with wine.
- Binary (not up to date):
  http://home.t-online.de/home/akemnade/freeciv-win-cvs.zip

Greetings
Andreas Kemnade