[Freeciv-Dev] Re: (PR#6380) remove is_real and is_valid sanity variables

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

John Wheeler wrote:
> [jdorje - Fri Oct  3 19:32:07 2003]:

>>Although I'm not sure what you have against assert?  It tells you what 
>>line the failure is on, which allows it to be easily tracked down.
> 
> 
> My main problem is that generic errors like assert(0) lead to poor RT
> tickets (once code is committed to CVS).  If someone puts something like
> "assert(0)" or "core dump" in the subject line, it is much less useful
> in searching for related problems.  And, if the problem is not solved
> right away, the line numbers can change, making the previously reported
> error message even more nebulous.

This patch replaces it with die() and a suitable error message.

Note that die() is a much stronger check than assert().

jason

Index: ai/aicity.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/aicity.c,v
retrieving revision 1.146
diff -u -r1.146 aicity.c
--- ai/aicity.c 2003/10/08 16:56:07     1.146
+++ ai/aicity.c 2003/10/10 19:51:31
@@ -663,7 +663,6 @@
   map_city_radius_iterate(pcity->x, pcity->y, x, y) {
     struct city *acity = map_get_tile(x,y)->worked;
     int city_map_x, city_map_y;
-    bool is_valid;
 
     if (acity && acity != pcity && acity->owner == pcity->owner)  {
       if (same_pos(acity->x, acity->y, x, y)) {
@@ -672,8 +671,9 @@
       }
       freelog(LOG_DEBUG, "%s taking over %s's square in (%d, %d)",
               pcity->name, acity->name, x, y);
-      is_valid = map_to_city_map(&city_map_x, &city_map_y, acity, x, y);
-      assert(is_valid);
+      if (!map_to_city_map(&city_map_x, &city_map_y, acity, x, y)) {
+       die("Bad city position (%d,%d) for %s.", x, y, acity->name);
+      }
       server_remove_worker_city(acity, city_map_x, city_map_y);
       acity->ppl_elvis++;
       if (!city_list_find_id(&minilist, acity->id)) {
Index: client/goto.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/goto.c,v
retrieving revision 1.60
diff -u -r1.60 goto.c
--- client/goto.c       2003/09/21 09:06:43     1.60
+++ client/goto.c       2003/10/10 19:51:31
@@ -544,7 +544,6 @@
 static unsigned char *get_drawn_char(int x, int y, enum direction8 dir)
 {
   int x1, y1;
-  bool is_real;
 
   assert(is_valid_dir(dir));
 
@@ -552,10 +551,10 @@
   assert(is_real_map_pos(x, y));
   normalize_map_pos(&x, &y);
 
-  is_real = MAPSTEP(x1, y1, x, y, dir);
-
-  /* It makes no sense to draw a goto line to a non-existent tile. */
-  assert(is_real);
+  if (!MAPSTEP(x1, y1, x, y, dir)) {
+    /* It makes no sense to draw a goto line to a non-existent tile. */
+    die("Bad map step from (%d,%d) in direction %d.", x, y, dir);
+  }
 
   if (dir >= 4) {
     x = x1;
Index: common/city.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/city.c,v
retrieving revision 1.198
diff -u -r1.198 city.c
--- common/city.c       2003/09/28 17:44:16     1.198
+++ common/city.c       2003/10/10 19:51:32
@@ -554,7 +554,6 @@
 int base_city_get_shields_tile(int x, int y, struct city *pcity,
                               bool is_celebrating)
 {
-  bool is_real;
   int map_x, map_y, s = 0;
   enum tile_special_type spec_t;
   enum tile_terrain_type tile_t;
@@ -562,8 +561,9 @@
   int before_penalty = (is_celebrating ? g->celeb_shields_before_penalty
                        : g->shields_before_penalty);
   
-  is_real = city_map_to_map(&map_x, &map_y, pcity, x, y);
-  assert(is_real);
+  if (!city_map_to_map(&map_x, &map_y, pcity, x, y)) {
+    die("Bad city position (%d,%d) for %s.", x, y, pcity->name);
+  }
 
   spec_t = map_get_special(map_x, map_y);
   tile_t = map_get_terrain(map_x, map_y);
@@ -652,11 +652,11 @@
   enum tile_special_type spec_t;
   enum tile_terrain_type tile_t;
   struct government *g = get_gov_pcity(pcity);
-  bool is_real;
   int map_x, map_y, t;
 
-  is_real = city_map_to_map(&map_x, &map_y, pcity, x, y);
-  assert(is_real);
+  if (!city_map_to_map(&map_x, &map_y, pcity, x, y)) {
+    die("Bad city position (%d,%d) for %s.", x, y, pcity->name);
+  }
 
   spec_t = map_get_special(map_x, map_y);
   tile_t = map_get_terrain(map_x, map_y);
@@ -755,7 +755,6 @@
 int base_city_get_food_tile(int x, int y, struct city *pcity,
                            bool is_celebrating)
 {
-  bool is_real;
   int map_x, map_y, f;
   enum tile_special_type spec_t;
   enum tile_terrain_type tile_t;
@@ -765,8 +764,9 @@
                        : g->food_before_penalty);
   bool city_auto_water;
 
-  is_real = city_map_to_map(&map_x, &map_y, pcity, x, y);
-  assert(is_real);
+  if (!city_map_to_map(&map_x, &map_y, pcity, x, y)) {
+    die("Bad city position (%d,%d) for %s.", x, y, pcity->name);
+  }
 
   spec_t = map_get_special(map_x, map_y);
   tile_t = map_get_terrain(map_x, map_y);
Index: server/citytools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/citytools.c,v
retrieving revision 1.236
diff -u -r1.236 citytools.c
--- server/citytools.c  2003/10/08 16:56:07     1.236
+++ server/citytools.c  2003/10/10 19:51:32
@@ -1957,20 +1957,18 @@
   /* Update adjacent cities. */
   {
     int map_x, map_y;
-    bool is_real;
 
-    is_real = city_map_to_map(&map_x, &map_y, pcity, city_x, city_y);
-    assert(is_real);
+    if (!city_map_to_map(&map_x, &map_y, pcity, city_x, city_y)) {
+      die("Bad city position (%d,%d) for %s.", city_x, city_y, pcity->name);
+    }
 
     map_city_radius_iterate(map_x, map_y, x1, y1) {
       struct city *pcity2 = map_get_city(x1, y1);
       if (pcity2 && pcity2 != pcity) {
        int city_x2, city_y2;
-       bool is_valid;
-
-       is_valid = map_to_city_map(&city_x2, &city_y2, pcity2, map_x,
-                                        map_y);
-       assert(is_valid);
+       if (!map_to_city_map(&city_x2, &city_y2, pcity2, map_x, map_y)) {
+         die("Bad map position (%d,%d) for %s.", map_x, map_y, pcity->name);
+       }
        update_city_tile_status(pcity2, city_x2, city_y2);
       }
     } map_city_radius_iterate_end;
@@ -2011,10 +2009,10 @@
 bool update_city_tile_status_map(struct city *pcity, int map_x, int map_y)
 {
   int city_x, city_y;
-  bool is_valid;
 
-  is_valid = map_to_city_map(&city_x, &city_y, pcity, map_x, map_y);
-  assert(is_valid);
+  if (!map_to_city_map(&city_x, &city_y, pcity, map_x, map_y)) {
+    die("Bad map position (%d,%d) for %s.", map_x, map_y, pcity->name);
+  }
   return update_city_tile_status(pcity, city_x, city_y);
 }
 
Index: server/gotohand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/gotohand.c,v
retrieving revision 1.175
diff -u -r1.175 gotohand.c
--- server/gotohand.c   2003/10/10 19:29:06     1.175
+++ server/gotohand.c   2003/10/10 19:51:32
@@ -1323,7 +1323,7 @@
   /* This has the side effect of marking the warmap with the possible paths */
   if (find_the_shortest_path(punit, waypoint_x, waypoint_y, restriction)) {
     do { /* move the unit along the path chosen by find_the_shortest_path() 
while we can */
-      bool last_tile, is_real, success;
+      bool last_tile, success;
       struct unit *penemy = NULL;
       int dir;
 
@@ -1340,8 +1340,10 @@
       }
 
       freelog(LOG_DEBUG, "Going %s", dir_get_name(dir));
-      is_real = MAPSTEP(x, y, punit->x, punit->y, dir);
-      assert(is_real);
+      if (!MAPSTEP(x, y, punit->x, punit->y, dir)) {
+       die("Bad map step from (%d,%d) in direction %d.",
+           punit->x, punit->y, dir);
+      }
 
       penemy = is_enemy_unit_tile(map_get_tile(x, y), unit_owner(punit));
       assert(punit->moves_left > 0);
Index: server/mapgen.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/mapgen.c,v
retrieving revision 1.118
diff -u -r1.118 mapgen.c
--- server/mapgen.c     2003/10/03 11:29:31     1.118
+++ server/mapgen.c     2003/10/10 19:51:32
@@ -1379,13 +1379,12 @@
                                               const struct gen234_state
                                               *const pstate)
 {
-  bool is_real;
-
   *x = pstate->w;
   *y = pstate->n;
 
-  is_real = normalize_map_pos(x, y);
-  assert(is_real);
+  if (!normalize_map_pos(x, y)) {
+    die("Bad map position (%d,%d).", *x, *y);
+  }
 
   assert((pstate->e - pstate->w) > 0);
   assert((pstate->e - pstate->w) < map.xsize);
@@ -1395,8 +1394,9 @@
   *x += myrand(pstate->e - pstate->w);
   *y += myrand(pstate->s - pstate->n);
 
-  is_real = normalize_map_pos(x, y);
-  assert(is_real);
+  if (!normalize_map_pos(x, y)) {
+    die("Bad map position (%d,%d).", *x, *y);
+  }
 }
 
 /**************************************************************************
@@ -1545,10 +1545,10 @@
       if (hmap(x, y) != 0) {
        int map_x = x + xo - pstate->w;
        int map_y = y + yo - pstate->n;
-       bool is_real;
 
-       is_real = normalize_map_pos(&map_x, &map_y);
-       assert(is_real);
+       if (!normalize_map_pos(&map_x, &map_y)) {
+         die("Bad map position (%d,%d).", map_x, map_y);
+       }
 
        checkmass--; 
        if(checkmass<=0) {
Index: server/sanitycheck.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/sanitycheck.c,v
retrieving revision 1.36
diff -u -r1.36 sanitycheck.c
--- server/sanitycheck.c        2003/10/09 00:46:02     1.36
+++ server/sanitycheck.c        2003/10/10 19:51:32
@@ -228,10 +228,10 @@
     if (ptile->worked) {
       struct city *pcity = ptile->worked;
       int city_x, city_y;
-      bool is_valid;
 
-      is_valid = map_to_city_map(&city_x, &city_y, pcity, x, y);
-      assert(is_valid);
+      if (!map_to_city_map(&city_x, &city_y, pcity, x, y)) {
+       die("Bad map position (%d,%d) for %s.", x, y, pcity->name);
+      }
 
       if (pcity->city_map[city_x][city_y] != C_TILE_WORKER) {
        freelog(LOG_ERROR, "%d,%d is listed as being worked by %s "
Index: server/savegame.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/savegame.c,v
retrieving revision 1.139
diff -u -r1.139 savegame.c
--- server/savegame.c   2003/10/07 18:55:09     1.139
+++ server/savegame.c   2003/10/10 19:51:32
@@ -935,10 +935,10 @@
                          C_TILE_EMPTY : C_TILE_UNAVAILABLE);
        } else if (*p=='1') {
          int map_x, map_y;
-         bool is_real;
 
-         is_real = city_map_to_map(&map_x, &map_y, pcity, x, y);
-         assert(is_real);
+         if (!city_map_to_map(&map_x, &map_y, pcity, x, y)) {
+           die("Bad city position (%d,%d) for %s.", x, y, pcity->name);
+         }
 
          if (map_get_tile(map_x, map_y)->worked) {
            /* oops, inconsistent savegame; minimal fix: */
@@ -1812,14 +1812,14 @@
     case C_TILE_WORKER:
       if (!res) {
        int map_x, map_y;
-       bool is_real;
 
        pcity->ppl_elvis++;
        set_worker_city(pcity, x, y, C_TILE_UNAVAILABLE);
        freelog(LOG_DEBUG, "Worked tile was unavailable!");
 
-       is_real = city_map_to_map(&map_x, &map_y, pcity, x, y);
-       assert(is_real);
+       if (!city_map_to_map(&map_x, &map_y, pcity, x, y)) {
+         die("Bad city position (%d,%d) for %s.", x, y, pcity->name);
+       }
 
        map_city_radius_iterate(map_x, map_y, x2, y2) {
          struct city *pcity2 = map_get_city(x2, y2);