[Freeciv-Dev] (PR#6203) server segfault with transporter code

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

Difficult to reproduce.

Apply the init_new_game.diff patch from the starting positions bug. 
This patch doesn't contribute to the bug (I assume), it basically just 
causes myrand() to be called a different number of times.

Then run an autogame with the attached rc.  Segfault occurs sometime 
after 1000 BC.

#0  0x080ba904 in unit_type (punit=0x0) at unittype.c:96
#1  0x080b8881 in is_ground_unit (punit=0x0) at unit.c:346
#2  0x08089889 in check_units () at sanitycheck.c:279
#3  0x08089a91 in sanity_check () at sanitycheck.c:327
#4  0x0804f274 in main_loop () at srv_main.c:1437
#5  0x0804fa5a in srv_loop () at srv_main.c:1800
#6  0x0804f4de in srv_main () at srv_main.c:1587
#7  0x0804a1e0 in main (argc=135242816, argv=0xbffffaa4) at civserver.c:154
#8  0x401a34ed in __libc_start_main () from /lib/libc.so.6

I see no way that this assertion could fail, however:
        assert(!is_ground_unit(find_unit_by_id(punit->transported_by)));

So I don't know...

jason

? core.10813
Index: server/gamehand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/gamehand.c,v
retrieving revision 1.127
diff -u -r1.127 gamehand.c
--- server/gamehand.c   2003/09/09 15:49:07     1.127
+++ server/gamehand.c   2003/09/19 17:02:56
@@ -32,39 +32,80 @@
 #include "gamehand.h"
 
 
-/**************************************************************************
-...
-**************************************************************************/
-void init_new_game(void)
+/****************************************************************************
+  Initialize the game.id variable to a random string of characters.
+****************************************************************************/
+static void init_game_id(void)
 {
   static const char chars[] =
       "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
-  int i, j, x, y;
-  int dx, dy;
-  Unit_Type_id utype;
-  int start_pos[MAX_NUM_PLAYERS]; /* indices into map.start_positions[] */
+  int i;
 
   for (i = 0; i < sizeof(game.id) - 1; i++) {
     game.id[i] = chars[myrand(sizeof(chars) - 1)];
   }
   game.id[i] = '\0';
+}
+
+/****************************************************************************
+  Place a starting unit for the player.
+****************************************************************************/
+static void place_starting_unit(int x, int y, struct player *pplayer,
+                               enum unit_flag_id role)
+{
+  Unit_Type_id utype;
+
+  assert(!is_non_allied_unit_tile(map_get_tile(x, y), pplayer));
+
+  /* For scenarios or dispersion, huts may coincide with player starts (in 
+   * other cases, huts are avoided as start positions).  Remove any such hut,
+   * and make sure to tell the client, since we may have already sent this
+   * tile (with the hut) earlier: */
+  if (map_has_special(x, y, S_HUT)) {
+    map_clear_special(x, y, S_HUT);
+    send_tile_info(NULL, x, y);
+    freelog(LOG_VERBOSE, "Removed hut on start position for %s",
+           pplayer->name);
+  }
+
+  /* Expose visible area. */
+  circle_iterate(x, y, game.rgame.init_vis_radius_sq, cx, cy) {
+    show_area(pplayer, cx, cy, 0);
+  } circle_iterate_end;
+
+  /* Create the unit of an appropriate type. */
+  utype = get_role_unit(role, 0);
+  (void) create_unit(pplayer, x, y, utype, FALSE, 0, -1);
+}
+
+/****************************************************************************
+  Initialize a new game: place the players' units onto the map, etc.
+****************************************************************************/
+void init_new_game(void)
+{
+  int start_pos[MAX_NUM_PLAYERS]; /* indices into map.start_positions[] */
+
+  init_game_id();
 
   if (!map.fixed_start_positions) {
     /* except in a scenario which provides them,
        shuffle the start positions around... */
-    assert(game.nplayers==map.num_start_positions);
-    for (i=0; i<game.nplayers;i++) { /* no advantage to the romans!! */
-      j=myrand(game.nplayers);
-      x=map.start_positions[j].x;
-      y=map.start_positions[j].y;
-      map.start_positions[j].x=map.start_positions[i].x;
-      map.start_positions[j].y=map.start_positions[i].y;
-      map.start_positions[i].x=x;
-      map.start_positions[i].y=y;
-    }
-    for(i=0; i<game.nplayers; i++) {
-      start_pos[i] = i;
-    } 
+    assert(game.nplayers == map.num_start_positions);
+    players_iterate(pplayer) {
+      /* no advantage to the romans!! */
+      int i = pplayer->player_no;
+      int j = myrand(game.nplayers);
+      int x = map.start_positions[j].x;
+      int y = map.start_positions[j].y;
+
+      map.start_positions[j].x = map.start_positions[i].x;
+      map.start_positions[j].y = map.start_positions[i].y;
+      map.start_positions[i].x = x;
+      map.start_positions[i].y = y;
+    } players_iterate_end;
+    players_iterate(pplayer) {
+      start_pos[pplayer->player_no] = pplayer->player_no;
+    } players_iterate_end;
   } else {
   /* In a scenario, choose starting positions by nation.
      If there are too few starts for number of nations, assign
@@ -75,82 +116,72 @@
     const int npos = map.num_start_positions;
     bool *pos_used = fc_calloc(npos, sizeof(bool));
     int nrem = npos;           /* remaining unused starts */
-    
-    for(i=0; i<game.nplayers; i++) {
-      int nation = game.players[i].nation;
+
+    players_iterate(pplayer) {
+      int nation = pplayer->nation;
+
       if (nation < npos) {
-       start_pos[i] = nation;
+       start_pos[pplayer->player_no] = nation;
        pos_used[nation] = TRUE;
        nrem--;
       } else {
-       start_pos[i] = npos;
+       start_pos[pplayer->player_no] = npos;
       }
-    }
-    for(i=0; i<game.nplayers; i++) {
-      if (start_pos[i] == npos) {
-       int k;
-       assert(nrem>0);
+    } players_iterate_end;
+
+    players_iterate(pplayer) {
+      if (start_pos[pplayer->player_no] == npos) {
+       int j, k;
+
+       assert(nrem > 0);
        k = myrand(nrem);
-       for(j=0; j<npos; j++) {
-         if (!pos_used[j] && (0==k--)) {
-           start_pos[i] = j;
+       for(j = 0; j < npos; j++) {
+         if (!pos_used[j] && (0 == k--)) {
+           start_pos[pplayer->player_no] = j;
            pos_used[j] = TRUE;
            nrem--;
            break;
          }
        }
-       assert(start_pos[i] != npos);
+       assert(start_pos[pplayer->player_no] != npos);
       }
-    }
+    } players_iterate_end;
     free(pos_used);
     pos_used = NULL;
   }
 
   /* Loop over all players, creating their initial units... */
-  for (i = 0; i < game.nplayers; i++) {
-    /* Start positions are warranted to be land. */
-    x = map.start_positions[start_pos[i]].x;
-    y = map.start_positions[start_pos[i]].y;
-    /* Loop over all initial units... */
-    for (j = 0; j < (game.settlers + game.explorer); j++) {
-      /* Determine a place to put the unit within the dispersion area.
-         (Always put first unit on start position.) */
-      if ((game.dispersion <= 0) || (j == 0)) {
-       dx = x;
-       dy = y;
-      } else {
-       bool is_real;
+  players_iterate(pplayer) {
+    int x = map.start_positions[start_pos[pplayer->player_no]].x;
+    int y = map.start_positions[start_pos[pplayer->player_no]].y;
 
-       do {
-         dx = x + myrand(2 * game.dispersion + 1) - game.dispersion;
-         dy = y + myrand(2 * game.dispersion + 1) - game.dispersion;
-         is_real = normalize_map_pos(&dx, &dy);
-       } while (!(is_real
-                  && map_get_continent(x, y) == map_get_continent(dx, dy)
-                  && !is_ocean(map_get_terrain(dx, dy))
-                  && !is_non_allied_unit_tile(map_get_tile(dx, dy),
-                                              get_player(i))));
-      }
-      /* For scenarios or dispersion, huts may coincide with player
-        starts (in other cases, huts are avoided as start positions).
-        Remove any such hut, and make sure to tell the client, since
-        we may have already sent this tile (with the hut) earlier:
-      */
-      if (map_has_special(dx, dy, S_HUT)) {
-        map_clear_special(dx, dy, S_HUT);
-       send_tile_info(NULL, dx, dy);
-        freelog(LOG_VERBOSE, "Removed hut on start position for %s",
-               game.players[i].name);
-      }
-      /* Expose visible area. */
-      circle_iterate(dx, dy, game.rgame.init_vis_radius_sq, cx, cy) {
-       show_area(&game.players[i], cx, cy, 0);
-      } circle_iterate_end;
+    /* Place the first unit. */
+    assert(game.settlers > 0);
+    place_starting_unit(x, y, pplayer, F_CITIES);
+  } players_iterate_end;
+
+  /* Place all other units. */
+  players_iterate(pplayer) {
+    int x = map.start_positions[start_pos[pplayer->player_no]].x;
+    int y = map.start_positions[start_pos[pplayer->player_no]].y;
+    int i, dx, dy;
+
+    for (i = 1; i < (game.settlers + game.explorer); i++) {
+      do {
+       dx = x + myrand(2 * game.dispersion + 1) - game.dispersion;
+       dy = y + myrand(2 * game.dispersion + 1) - game.dispersion;
+      } while (!(normalize_map_pos(&dx, &dy)
+                && map_get_continent(x, y) == map_get_continent(dx, dy)
+                && !is_ocean(map_get_terrain(dx, dy))
+                && !is_non_allied_unit_tile(map_get_tile(dx, dy),
+                                            pplayer)));
+
+
       /* Create the unit of an appropriate type. */
-      utype = get_role_unit((j < game.settlers) ? F_CITIES : L_EXPLORER, 0);
-      (void) create_unit(&game.players[i], dx, dy, utype, FALSE, 0, -1);
+      place_starting_unit(dx, dy, pplayer,
+                         (i < game.settlers) ? F_CITIES : L_EXPLORER);
     }
-  }
+  } players_iterate_end;
 
   /* Initialise list of improvements with world-wide equiv_range */
   improvement_status_init(game.improvements, ARRAY_SIZE(game.improvements));
Index: server/sanitycheck.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/sanitycheck.c,v
retrieving revision 1.32
diff -u -r1.32 sanitycheck.c
--- server/sanitycheck.c        2003/09/19 14:14:45     1.32
+++ server/sanitycheck.c        2003/09/19 17:02:56
@@ -119,6 +119,14 @@
 
     unit_list_iterate(ptile->units, punit) {
       assert(same_pos(punit->x, punit->y, x, y));
+
+      /* Check diplomatic status of stacked units. */
+      unit_list_iterate(ptile->units, punit2) {
+       assert(pplayers_allied(unit_owner(punit), unit_owner(punit2)));
+      } unit_list_iterate_end;
+      if (pcity) {
+       assert(pplayers_allied(unit_owner(punit), city_owner(pcity)));
+      }
     } unit_list_iterate_end;
   } whole_map_iterate_end;
 }

set seed 213880319
set randseed 1063990734
hard
set timeout -1
set aifill 30
create b

start