[Freeciv-Dev] Re: Reproducable core dump (PR#1051)

[jdorje@xxxxxxxxxxxxxxxxxxxxx]

"Pieter J. Kersten" wrote:
> 
> L.S.,
> 
> I've found a reproducable core dump in the latest CVS version. Attached is
> a savegame file. Login with 'pieter'. The crash happens when you try to
> send the stealth bomber in Lisboa to Alcochete with a Goto. It exits with
> an assert error:  map_is_real or something similar.

The air goto code does not check the realness of the position or
normalize it; it counts on map_get_tile to do this.  map_get_tile no
longer does this.

Attached is a possible fix.

jason

? rc
? diff
? old
? topology
? corecleanup_08.ReadMe
? 20011101.gz
Index: common/map.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/map.h,v
retrieving revision 1.101
diff -u -r1.101 map.h
--- common/map.h        2001/10/30 10:59:20     1.101
+++ common/map.h        2001/11/02 20:09:54
@@ -217,6 +217,7 @@
 /*
  * Returns true if the step yields a new valid map position. If yes
  * (dest_x, dest_y) is set to the new map position.
+ * WARNING: MAPSTEP(x, y, x, y, dir) is not safe!
  */
 #define MAPSTEP(dest_x, dest_y, src_x, src_y, dir)     \
 (    DIRSTEP(dest_x, dest_y, dir),                     \
Index: server/gotohand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/gotohand.c,v
retrieving revision 1.122
diff -u -r1.122 gotohand.c
--- server/gotohand.c   2001/10/08 12:11:17     1.122
+++ server/gotohand.c   2001/11/02 20:09:56
@@ -1442,7 +1442,7 @@
 int air_can_move_between(int moves, int src_x, int src_y,
                         int dest_x, int dest_y, struct player *pplayer)
 {
-  int x, y, go_x, go_y, i, movescount;
+  int x, y, movescount, preferred_dir, best_dir, best;
   struct tile *ptile;
   freelog(LOG_DEBUG, "naive_air_can_move_between %i,%i -> %i,%i, moves: %i",
          src_x, src_y, dest_x, dest_y, moves);
@@ -1458,61 +1458,34 @@
     if (movescount <= 1)
       goto TRYFULL;
 
-    go_x = (x > dest_x ?
-           (x-dest_x < map.xsize/2 ? -1 : 1) :
-           (dest_x-x < map.xsize/2 ? 1 : -1));
-    go_y = (dest_y > y ? 1 : -1);
-
-    freelog(LOG_DEBUG, "%i,%i to %i,%i. go_x: %i, go_y:%i",
-           x, y, dest_x, dest_y, go_x, go_y);
-    if (x == dest_x) {
-      for (i = x-1 ; i <= x+1; i++)
-       if ((ptile = map_get_tile(i, y+go_y))
-           /* && is_real_tile(i, y+go_y) */
-           && ! is_non_allied_unit_tile(ptile, pplayer)) {
-         x = i;
-         y += go_y;
-         goto NEXTCYCLE;
-       }
-      goto TRYFULL;
-    } else if (y == dest_y) {
-      for (i = y-1 ; i <= y+1; i++)
-       if ((ptile = map_get_tile(x+go_x, i))
-           && is_real_tile(x+go_x, i)
-           && ! is_non_allied_unit_tile(ptile, pplayer)) {
-         x += go_x;
-         y = i;
-         goto NEXTCYCLE;
-       }
-      goto TRYFULL;
-    }
-
-    /* (x+go_x, y+go_y) is always real, given (x, y) is real */
-    ptile = map_get_tile(x+go_x, y+go_y);
-    if (! is_non_allied_unit_tile(ptile, pplayer)) {
-      x += go_x;
-      y += go_y;
-      goto NEXTCYCLE;
-    }
+    preferred_dir = straightest_direction(x, y, dest_x, dest_y);
 
-    /* (x+go_x, y) is always real, given (x, y) is real */
-    ptile = map_get_tile(x+go_x, y);
-    if (! is_non_allied_unit_tile(ptile, pplayer)) {
-      x += go_x;
-      goto NEXTCYCLE;
-    }
+    freelog(LOG_DEBUG, "%i,%i to %i,%i",
+           x, y, dest_x, dest_y);
+    best_dir = -1, best = -1;
+    adjc_dir_iterate(x, y, nx, ny, dir) {
+      int value;
+      if (!dir_ok(x, y, dest_x, dest_y, dir))
+        continue;
+      ptile = map_get_tile(nx, ny);
+      if (is_non_allied_unit_tile(ptile, pplayer))
+        continue;
+      value = DIR_DX[preferred_dir] * DIR_DX[dir] + DIR_DY[preferred_dir] * 
DIR_DY[dir];
+      if (value > best) {
+        best = value;
+        best_dir = dir;
+      }
+    } adjc_dir_iterate_end;
 
-    /* (x, y+go_y) is always real, given (x, y) is real */
-    ptile = map_get_tile(x, y+go_y);
-    if (! is_non_allied_unit_tile(ptile, pplayer)) {
-      y += go_y;
-      goto NEXTCYCLE;
+    if (best_dir == -1)
+      goto TRYFULL;
+    else {
+      int nx, ny, is_real;
+      is_real = MAPSTEP(nx, ny, x, y, best_dir);
+      assert(is_real);
+      x = nx, y = ny;
     }
 
-    /* we didn't advance.*/
-    goto TRYFULL;
-
-  NEXTCYCLE:
     movescount--;
   }
   /* if this loop stopped we made it! We found a way! */