[Freeciv-Dev] (PR#6464) better sanity_check_city() function

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

This patch moves sanity_check_city into sanitycheck.c, and moves almost 
all of the city checks from check_cities() into that function.

This should be of use in debugging.

We might want to consider making die() non-fatal if NDEBUG is defined, 
and using it instead of freelog() or assert() in the sanity checks.

jason

Index: server/cityhand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/cityhand.c,v
retrieving revision 1.123
diff -u -r1.123 cityhand.c
--- server/cityhand.c   2003/09/20 19:24:54     1.123
+++ server/cityhand.c   2003/10/10 05:53:14
@@ -35,6 +35,7 @@
 #include "citytools.h"
 #include "cityturn.h"
 #include "plrhand.h"
+#include "sanitycheck.h"
 
 #include "cityhand.h"
 
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 05:53:14
@@ -40,6 +40,7 @@
 #include "gamelog.h"
 #include "maphand.h"
 #include "plrhand.h"
+#include "sanitycheck.h"
 #include "sernet.h"
 #include "settlers.h"
 #include "spacerace.h"
Index: server/cityturn.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/cityturn.c,v
retrieving revision 1.227
diff -u -r1.227 cityturn.c
--- server/cityturn.c   2003/10/08 16:56:07     1.227
+++ server/cityturn.c   2003/10/10 05:53:14
@@ -39,6 +39,7 @@
 #include "gamelog.h"
 #include "maphand.h"
 #include "plrhand.h"
+#include "sanitycheck.h"
 #include "settlers.h"
 #include "spacerace.h"
 #include "srv_main.h"
@@ -139,25 +140,6 @@
   city_list_iterate(pplayer->cities, pcity) {
     remove_obsolete_buildings_city(pcity, FALSE);
   } city_list_iterate_end;
-}
-
-/**************************************************************************
-  Hard check that the numbers of city workers/specialists add up.
-**************************************************************************/
-void real_sanity_check_city(struct city *pcity, const char *file, int line)
-{
-  int worker = 0;
-
-  city_map_iterate(x, y) {
-    if (get_worker_city(pcity, x, y) == C_TILE_WORKER) {
-      worker++;
-    }
-  } city_map_iterate_end;
-  if (worker + city_specialists(pcity) != pcity->size + 1) {
-    die("%s is illegal (size%d w%d e%d t%d s%d) in %s line %d",
-        pcity->name, pcity->size, worker, pcity->ppl_elvis,
-        pcity->ppl_taxman, pcity->ppl_scientist, file, line);
-  }
 }
 
 /**************************************************************************
Index: server/cityturn.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/cityturn.h,v
retrieving revision 1.32
diff -u -r1.32 cityturn.h
--- server/cityturn.h   2003/09/22 16:04:03     1.32
+++ server/cityturn.h   2003/10/10 05:53:14
@@ -21,9 +21,6 @@
 struct unit;
 struct conn_list;
 
-#define sanity_check_city(x) real_sanity_check_city(x, __FILE__, __LINE__)
-void real_sanity_check_city(struct city *pcity, const char *file, int line);
-
 void city_refresh(struct city *pcity);          /* call if city has changed */
 void global_city_refresh(struct player *pplayer); /* tax/govt changed */
 
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 05:53:14
@@ -133,6 +133,107 @@
 }
 
 /**************************************************************************
+  Verify that the city has sane values.
+**************************************************************************/
+void real_sanity_check_city(struct city *pcity, const char *file, int line)
+{
+  int workers = 0;
+  struct player *pplayer = city_owner(pcity);
+
+  assert(pcity->size >= 1);
+  assert(is_normal_map_pos(pcity->x, pcity->y));
+
+  unit_list_iterate(pcity->units_supported, punit) {
+    assert(punit->homecity == pcity->id);
+    assert(unit_owner(punit) == pplayer);
+  } unit_list_iterate_end;
+
+  /* Note that cities may be found on land or water. */
+
+  city_map_iterate(x, y) {
+    int map_x, map_y;
+
+    if (city_map_to_map(&map_x, &map_y, pcity, x, y)) {
+      struct tile *ptile = map_get_tile(map_x, map_y);
+      struct player *owner = map_get_owner(map_x, map_y);
+
+      switch (get_worker_city(pcity, x, y)) {
+      case C_TILE_EMPTY:
+       if (map_get_tile(map_x, map_y)->worked) {
+         freelog(LOG_ERROR, "Tile at %s->%d,%d marked as "
+                 "empty but worked by %s!",
+                 pcity->name, x, y,
+                 map_get_tile(map_x, map_y)->worked->name);
+       }
+       if (is_enemy_unit_tile(ptile, pplayer)) {
+         freelog(LOG_ERROR, "Tile at %s->%d,%d marked as "
+                 "empty but occupied by an enemy unit!",
+                 pcity->name, x, y);
+       }
+       if (game.borders > 0
+           && owner && owner->player_no != pcity->owner) {
+         freelog(LOG_ERROR, "Tile at %s->%d,%d marked as "
+                 "empty but in enemy territory!",
+                 pcity->name, x, y);
+       }
+       if (!city_can_work_tile(pcity, x, y)) {
+         /* Complete check. */
+         freelog(LOG_ERROR, "Tile at %s->%d,%d marked as "
+                 "empty but is unavailable!",
+                 pcity->name, x, y);
+       }
+       break;
+      case C_TILE_WORKER:
+       if (map_get_tile(map_x, map_y)->worked != pcity) {
+         freelog(LOG_ERROR, "Tile at %s->%d,%d marked as "
+                 "worked but main map disagrees!",
+                 pcity->name, x, y);
+       }
+       if (is_enemy_unit_tile(ptile, pplayer)) {
+         freelog(LOG_ERROR, "Tile at %s->%d,%d marked as "
+                 "worked but occupied by an enemy unit!",
+                 pcity->name, x, y);
+       }
+       if (game.borders > 0
+           && owner && owner->player_no != pcity->owner) {
+         freelog(LOG_ERROR, "Tile at %s->%d,%d marked as "
+                 "worked but in enemy territory!",
+                 pcity->name, x, y);
+       }
+       if (!city_can_work_tile(pcity, x, y)) {
+         /* Complete check. */
+         freelog(LOG_ERROR, "Tile at %s->%d,%d marked as "
+                 "worked but is unavailable!",
+                 pcity->name, x, y);
+       }
+       break;
+      case C_TILE_UNAVAILABLE:
+       if (city_can_work_tile(pcity, x, y)) {
+         freelog(LOG_ERROR, "Tile at %s->%d,%d marked as "
+                 "unavailable but seems to be available!",
+                 pcity->name, x, y);
+       }
+       break;
+      }
+    } else {
+      assert(get_worker_city(pcity, x, y) == C_TILE_UNAVAILABLE);
+    }
+  } city_map_iterate_end;
+
+  /* Sanity check city size versus worker and specialist counts. */
+  city_map_iterate(x, y) {
+    if (get_worker_city(pcity, x, y) == C_TILE_WORKER) {
+      workers++;
+    }
+  } city_map_iterate_end;
+  if (workers + city_specialists(pcity) != pcity->size + 1) {
+    die("%s is illegal (size%d w%d e%d t%d s%d) in %s line %d",
+        pcity->name, pcity->size, workers, pcity->ppl_elvis,
+        pcity->ppl_taxman, pcity->ppl_scientist, file, line);
+  }
+}
+
+/**************************************************************************
 ...
 **************************************************************************/
 static void check_cities(void)
@@ -140,86 +241,8 @@
   players_iterate(pplayer) {
     city_list_iterate(pplayer->cities, pcity) {
       assert(city_owner(pcity) == pplayer);
-      assert(pcity->size >= 1);
 
-      unit_list_iterate(pcity->units_supported, punit) {
-       assert(punit->homecity == pcity->id);
-       assert(unit_owner(punit) == pplayer);
-      } unit_list_iterate_end;
-
-      assert(is_normal_map_pos(pcity->x, pcity->y));
-
-      /* Note that cities may be found on land or water. */
-
-      city_map_iterate(x, y) {
-       int map_x,map_y;
-
-       if (city_map_to_map(&map_x, &map_y, pcity, x, y)) {
-         struct tile *ptile = map_get_tile(map_x, map_y);
-         struct player *owner = map_get_owner(map_x, map_y);
-
-         switch (get_worker_city(pcity, x, y)) {
-         case C_TILE_EMPTY:
-           if (map_get_tile(map_x, map_y)->worked) {
-             freelog(LOG_ERROR, "Tile at %s->%d,%d marked as "
-                     "empty but worked by %s!",
-                     pcity->name, x, y,
-                     map_get_tile(map_x, map_y)->worked->name);
-           }
-           if (is_enemy_unit_tile(ptile, pplayer)) {
-             freelog(LOG_ERROR, "Tile at %s->%d,%d marked as "
-                     "empty but occupied by an enemy unit!",
-                     pcity->name, x, y);
-           }
-           if (game.borders > 0
-               && owner && owner->player_no != pcity->owner) {
-             freelog(LOG_ERROR, "Tile at %s->%d,%d marked as "
-                     "empty but in enemy territory!",
-                     pcity->name, x, y);
-           }
-           if (!city_can_work_tile(pcity, x, y)) {
-             /* Complete check. */
-             freelog(LOG_ERROR, "Tile at %s->%d,%d marked as "
-                     "empty but is unavailable!",
-                     pcity->name, x, y);
-           }
-           break;
-         case C_TILE_WORKER:
-           if (map_get_tile(map_x, map_y)->worked != pcity) {
-             freelog(LOG_ERROR, "Tile at %s->%d,%d marked as "
-                     "worked but main map disagrees!",
-                     pcity->name, x, y);
-           }
-           if (is_enemy_unit_tile(ptile, pplayer)) {
-             freelog(LOG_ERROR, "Tile at %s->%d,%d marked as "
-                     "worked but occupied by an enemy unit!",
-                     pcity->name, x, y);
-           }
-           if (game.borders > 0
-                && owner && owner->player_no != pcity->owner) {
-             freelog(LOG_ERROR, "Tile at %s->%d,%d marked as "
-                     "worked but in enemy territory!",
-                     pcity->name, x, y);
-           }
-           if (!city_can_work_tile(pcity, x, y)) {
-             /* Complete check. */
-             freelog(LOG_ERROR, "Tile at %s->%d,%d marked as "
-                     "worked but is unavailable!",
-                     pcity->name, x, y);
-           }
-           break;
-         case C_TILE_UNAVAILABLE:
-           if (city_can_work_tile(pcity, x, y)) {
-             freelog(LOG_ERROR, "Tile at %s->%d,%d marked as "
-                     "unavailable but seems to be available!",
-                     pcity->name, x, y);
-           }
-           break;
-         }
-       } else {
-         assert(get_worker_city(pcity, x, y) == C_TILE_UNAVAILABLE);
-       }
-      } city_map_iterate_end;
+      sanity_check_city(pcity);
     } city_list_iterate_end;
   } players_iterate_end;
 
Index: server/sanitycheck.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/sanitycheck.h,v
retrieving revision 1.1
diff -u -r1.1 sanitycheck.h
--- server/sanitycheck.h        2001/04/06 11:29:01     1.1
+++ server/sanitycheck.h        2003/10/10 05:53:14
@@ -13,6 +13,9 @@
 #ifndef FC__SANITYCHECK_H
 #define FC__SANITYCHECK_H
 
+#define sanity_check_city(x) real_sanity_check_city(x, __FILE__, __LINE__)
+void real_sanity_check_city(struct city *pcity, const char *file, int line);
+
 void sanity_check(void);
 
 #endif  /* FC__SANITYCHECK_H */