[Freeciv-Dev] (PR#10392) uninitialized data in PF code

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

<URL: http://rt.freeciv.org/Ticket/Display.html?id=10392 >

With the attached autogame I get this valgrind warning:

#0  0x08135b90 in fill_position (pf_map=0x1baff4e8, ptile=0x1c380328,
     pos=0x1be84bf0) at path_finding.c:547
#1  0x08135e90 in construct_path (pf_map=0x1baff4e8, dest_tile=0x1c381ae8)
     at path_finding.c:641
#2  0x08135fd3 in pf_get_path (pf_map=0x1baff4e8, ptile=0x1c381ae8)
     at path_finding.c:684
#3  0x081299da in find_rampage_target (punit=0x1bb10730, thresh_adj=1,
     thresh_move=1) at aiunit.c:717
#4  0x08129b6b in ai_military_rampage (punit=0x1bb10730, thresh_adj=1,
     thresh_move=1) at aiunit.c:752
#5  0x0812cc2c in ai_military_attack (pplayer=0x8287d30, punit=0x1bb10730)
     at aiunit.c:1769
#6  0x0812d847 in ai_manage_military (pplayer=0x8287d30, punit=0x1bb10730)
     at aiunit.c:2050
#7  0x0812de2e in ai_manage_unit (pplayer=0x8287d30, punit=0x1bb10730)
     at aiunit.c:2189
#8  0x0812df72 in ai_manage_units (pplayer=0x8287d30) at aiunit.c:2210
#9  0x08122196 in ai_do_first_activities (pplayer=0x8287d30) at aihand.c:368
#10 0x0804f6e0 in ai_start_turn () at srv_main.c:468
#11 0x0804f95e in begin_phase (is_new_phase=true) at srv_main.c:540
#12 0x0805171c in main_loop () at srv_main.c:1507
#13 0x0805204e in srv_loop () at srv_main.c:1875
#14 0x08051994 in srv_main () at srv_main.c:1626
#15 0x0804a47a in main (argc=1, argv=0x52bfe7e4) at civserver.c:170

the reason is that in constructing a path back from the target tile to 
the source tile, the PF code calls mapstep one time too many.  Even if 
we're on the last tile it still calls mapstep, stepping into an 
unitialized direction.

Currently this is probably harmless since is_valid_dir filters it out. 
Unless it happes to be a valid direction, in which case it is still 
harmless.  However it's not good in general and could break an assertion 
someone might add someday.

This patch fixes it in two ways:

- Initialize the initial node's direction to -1.
- Don't go backwards from the source node when constructing a path.

jason

> create b
> set aifill 5
> set timeout -1
> set saveturns 1
> hard

> set mapseed 1052689428
> set gameseed 1096432495

> start

Index: common/aicore/path_finding.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/aicore/path_finding.c,v
retrieving revision 1.23
diff -u -r1.23 path_finding.c
--- common/aicore/path_finding.c        29 Sep 2004 02:24:23 -0000      1.23
+++ common/aicore/path_finding.c        30 Sep 2004 03:03:26 -0000
@@ -464,6 +464,7 @@
   pf_map->lattice[pf_map->tile->index].cost = pf_map->params->move_rate
       - pf_map->params->moves_left_initially;
   pf_map->lattice[pf_map->tile->index].extra_cost = 0;
+  pf_map->lattice[pf_map->tile->index].dir_to_here = -1;
   if (pf_map->params->is_pos_dangerous) {
     /* The starting point is safe */
     pf_map->d_lattice[pf_map->tile->index].is_dangerous = FALSE;
@@ -638,8 +639,10 @@
 
     dir_next = node->dir_to_here;
 
-    /* Step further back, if we haven't finished yet */
-    ptile = mapstep(ptile, DIR_REVERSE(dir_next));
+    if (i > 0) {
+      /* Step further back, if we haven't finished yet */
+      ptile = mapstep(ptile, DIR_REVERSE(dir_next));
+    }
   }
 
   return path;