[Freeciv-Dev] (PR#13516) make improvement_types[] static

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

<URL: http://bugs.freeciv.org/Ticket/Display.html?id=13516 >

This patch makes the improvement_types[] array in improvement.c static,
and changes all former users to use get_improvement_type or another
accessor.

Also get_improvement_type now does bounds checking instead of
arbitrarily returning random memory if given incorrect input.

-jason

Index: ai/advmilitary.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/advmilitary.c,v
retrieving revision 1.193
diff -p -u -r1.193 advmilitary.c
--- ai/advmilitary.c    22 Jul 2005 16:18:04 -0000      1.193
+++ ai/advmilitary.c    23 Jul 2005 00:10:00 -0000
@@ -1458,6 +1458,6 @@ void military_advisor_choose_build(struc
              unit_types[choice->choice].name, choice->want);
   } else {
     CITY_LOG(LOGLEVEL_BUILD, pcity, "military advisor choice: %s (want %d)",
-             improvement_types[choice->choice].name, choice->want);
+             get_improvement_name(choice->choice), choice->want);
   }
 }
Index: client/climisc.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/climisc.c,v
retrieving revision 1.170
diff -p -u -r1.170 climisc.c
--- client/climisc.c    22 Jul 2005 16:18:04 -0000      1.170
+++ client/climisc.c    23 Jul 2005 00:10:01 -0000
@@ -1077,7 +1077,7 @@ void cityrep_buy(struct city *pcity)
     assert(!pcity->is_building_unit);
     my_snprintf(buf, sizeof(buf),
                _("You don't buy %s in %s!"),
-               improvement_types[pcity->currently_building].name,
+               get_improvement_name(pcity->currently_building),
                pcity->name);
     append_output_window(buf);
     return;
Index: client/helpdata.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/helpdata.c,v
retrieving revision 1.120
diff -p -u -r1.120 helpdata.c
--- client/helpdata.c   22 Jul 2005 16:18:04 -0000      1.120
+++ client/helpdata.c   23 Jul 2005 00:10:01 -0000
@@ -418,7 +418,7 @@ void boot_help_texts(void)
            if (improvement_exists(i) && !is_great_wonder(i)) {
              pitem = new_help_item(current_type);
              my_snprintf(name, sizeof(name), " %s",
-                         improvement_types[i].name);
+                         get_improvement_name(i));
              pitem->topic = mystrdup(name);
              pitem->text = mystrdup("");
              help_list_append(category_nodes, pitem);
@@ -429,7 +429,7 @@ void boot_help_texts(void)
            if (improvement_exists(i) && is_great_wonder(i)) {
              pitem = new_help_item(current_type);
              my_snprintf(name, sizeof(name), " %s",
-                         improvement_types[i].name);
+                         get_improvement_name(i));
              pitem->topic = mystrdup(name);
              pitem->text = mystrdup("");
              help_list_append(category_nodes, pitem);
@@ -644,18 +644,18 @@ char *helptext_building(char *buf, size_
     return buf;
   }
 
-  imp = &improvement_types[which];
+  imp = get_improvement_type(which);
   
   if (imp->helptext && imp->helptext[0] != '\0') {
     cat_snprintf(buf, bufsz, "%s\n\n", _(imp->helptext));
   }
 
-  if (tech_exists(improvement_types[which].obsolete_by)) {
+  if (tech_exists(get_improvement_type(which)->obsolete_by)) {
     cat_snprintf(buf, bufsz,
                 _("* The discovery of %s will make %s obsolete.\n"),
                 get_tech_name(game.player_ptr,
-                              improvement_types[which].obsolete_by),
-                improvement_types[which].name);
+                              get_improvement_type(which)->obsolete_by),
+                get_improvement_type(which)->name);
   }
 
   if (building_has_effect(which, EFT_ENABLE_NUKE)
Index: client/packhand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/packhand.c,v
retrieving revision 1.537
diff -p -u -r1.537 packhand.c
--- client/packhand.c   22 Jul 2005 16:18:05 -0000      1.537
+++ client/packhand.c   23 Jul 2005 00:10:01 -0000
@@ -2140,16 +2140,15 @@ void handle_ruleset_tech(struct packet_r
 **************************************************************************/
 void handle_ruleset_building(struct packet_ruleset_building *p)
 {
-  struct impr_type *b;
+  struct impr_type *b = get_improvement_type(p->id);
   int i;
 
-  if(p->id < 0 || p->id >= game.control.num_impr_types || p->id >= B_LAST) {
+  if (!b) {
     freelog(LOG_ERROR,
            "Received bad building id %d in handle_ruleset_building()",
            p->id);
     return;
   }
-  b = &improvement_types[p->id];
 
   b->genus = p->genus;
   sz_strlcpy(b->name_orig, p->name);
@@ -2172,7 +2171,7 @@ void handle_ruleset_building(struct pack
 #ifdef DEBUG
   if(p->id == game.control.num_impr_types-1) {
     impr_type_iterate(id) {
-      b = &improvement_types[id];
+      b = get_improvement_type(id);
       freelog(LOG_DEBUG, "Impr: %s...",
              b->name);
       if (tech_exists(b->obsolete_by)) {
Index: client/gui-gtk-2.0/helpdlg.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk-2.0/helpdlg.c,v
retrieving revision 1.56
diff -p -u -r1.56 helpdlg.c
--- client/gui-gtk-2.0/helpdlg.c        22 Jul 2005 16:18:05 -0000      1.56
+++ client/gui-gtk-2.0/helpdlg.c        23 Jul 2005 00:10:02 -0000
@@ -690,7 +690,7 @@ static void help_update_improvement(cons
   create_help_page(HELP_IMPROVEMENT);
   
   if (which<game.control.num_impr_types) {
-    struct impr_type *imp = &improvement_types[which];
+    struct impr_type *imp = get_improvement_type(which);
     int i;
     char req_buf[512];
 
@@ -737,7 +737,7 @@ static void help_update_wonder(const str
   create_help_page(HELP_WONDER);
 
   if (which<game.control.num_impr_types) {
-    struct impr_type *imp = &improvement_types[which];
+    struct impr_type *imp = get_improvement_type(which);
     int i;
     char req_buf[512];
 
@@ -934,26 +934,26 @@ static void help_update_tech(const struc
     impr_type_iterate(j) {
       /* FIXME: need a more general mechanism for this, since this
        * helptext needs to be shown in all possible req source types. */
-      requirement_vector_iterate(&improvement_types[j].reqs, preq) {
+      requirement_vector_iterate(&get_improvement_type(j)->reqs, preq) {
        if (preq->source.type == REQ_BUILDING
            && preq->source.value.building == i) {
          hbox = gtk_hbox_new(FALSE, 0);
          gtk_container_add(GTK_CONTAINER(help_vbox), hbox);
          w = gtk_label_new(_("Allows"));
          gtk_box_pack_start(GTK_BOX(hbox), w, FALSE, FALSE, 0);
-         w = help_slink_new(improvement_types[j].name,
+         w = help_slink_new(get_improvement_name(j),
              is_great_wonder(j) ? HELP_WONDER
              : HELP_IMPROVEMENT);
          gtk_box_pack_start(GTK_BOX(hbox), w, FALSE, FALSE, 0);
          gtk_widget_show_all(hbox);
        }
       } requirement_vector_iterate_end;
-      if(i==improvement_types[j].obsolete_by) {
+      if (i == get_improvement_type(j)->obsolete_by) {
         hbox = gtk_hbox_new(FALSE, 0);
         gtk_container_add(GTK_CONTAINER(help_vbox), hbox);
         w = gtk_label_new(_("Obsoletes"));
         gtk_box_pack_start(GTK_BOX(hbox), w, FALSE, FALSE, 0);
-        w = help_slink_new(improvement_types[j].name,
+        w = help_slink_new(get_improvement_name(j),
                          is_great_wonder(j) ? HELP_WONDER : HELP_IMPROVEMENT);
         gtk_box_pack_start(GTK_BOX(hbox), w, FALSE, FALSE, 0);
         gtk_widget_show_all(hbox);
Index: common/city.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/city.c,v
retrieving revision 1.357
diff -p -u -r1.357 city.c
--- common/city.c       22 Jul 2005 16:18:05 -0000      1.357
+++ common/city.c       23 Jul 2005 00:10:02 -0000
@@ -554,16 +554,19 @@ bool city_got_building(const struct city
 **************************************************************************/
 int improvement_upkeep(const struct city *pcity, Impr_type_id i) 
 {
+  int upkeep;
+
   if (!improvement_exists(i))
     return 0;
   if (is_wonder(i))
     return 0;
-  if (improvement_types[i].upkeep
-      <= get_building_bonus(pcity, i, EFT_UPKEEP_FREE)) {
+
+  upkeep = get_improvement_type(i)->upkeep;
+  if (upkeep <= get_building_bonus(pcity, i, EFT_UPKEEP_FREE)) {
     return 0;
   }
   
-  return (improvement_types[i].upkeep);
+  return upkeep;
 }
 
 /**************************************************************************
@@ -2299,7 +2302,7 @@ void city_add_improvement(struct city *p
 void city_remove_improvement(struct city *pcity, Impr_type_id impr)
 {
   freelog(LOG_DEBUG,"Improvement %s removed from city %s",
-          improvement_types[impr].name, pcity->name);
+          get_improvement_name(impr), pcity->name);
   
   pcity->improvements[impr] = I_NONE;
 }
Index: common/game.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/game.c,v
retrieving revision 1.231
diff -p -u -r1.231 game.c
--- common/game.c       22 Jul 2005 16:18:05 -0000      1.231
+++ common/game.c       23 Jul 2005 00:10:02 -0000
@@ -569,7 +569,7 @@ void translate_data_names(void)
   } unit_type_iterate_end;
 
   impr_type_iterate(i) {
-    struct impr_type *tthis = &improvement_types[i];
+    struct impr_type *tthis = get_improvement_type(i);
 
     tthis->name = Q_(tthis->name_orig);
   } impr_type_iterate_end;
Index: common/improvement.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/improvement.c,v
retrieving revision 1.66
diff -p -u -r1.66 improvement.c
--- common/improvement.c        8 Jul 2005 03:31:18 -0000       1.66
+++ common/improvement.c        23 Jul 2005 00:10:02 -0000
@@ -44,7 +44,7 @@ The improvement_types array is now setup
    server/ruleset.c (for the server)
    client/packhand.c (for the client)
 **************************************************************************/
-struct impr_type improvement_types[B_LAST];
+static struct impr_type improvement_types[B_LAST];
 
 /**************************************************************************
   Convert impr genus names to enum; case insensitive;
@@ -70,7 +70,9 @@ void improvements_init(void)
   int i;
 
   for (i = 0; i < ARRAY_SIZE(improvement_types); i++) {
-    struct impr_type *p = get_improvement_type(i);
+    /* Can't use get_improvment_type here because num_impr_types isn't
+     * set yet. */
+    struct impr_type *p = &improvement_types[i];
 
     p->index = i;
     requirement_vector_init(&p->reqs);
@@ -130,6 +132,9 @@ bool improvement_exists(Impr_type_id id)
 **************************************************************************/
 struct impr_type *get_improvement_type(Impr_type_id id)
 {
+  if (id < 0 || id > game.control.num_impr_types) {
+    return NULL;
+  }
   return &improvement_types[id];
 }
 
@@ -154,7 +159,7 @@ const char *get_improvement_name_orig(Im
 ****************************************************************************/
 int impr_build_shield_cost(Impr_type_id id)
 {
-  return improvement_types[id].build_cost;
+  return get_improvement_type(id)->build_cost;
 }
 
 /****************************************************************************
@@ -162,8 +167,9 @@ int impr_build_shield_cost(Impr_type_id 
 ****************************************************************************/
 int impr_buy_gold_cost(Impr_type_id id, int shields_in_stock)
 {
-  int cost = 0, missing =
-      improvement_types[id].build_cost - shields_in_stock;
+  int cost = 0;
+  const int missing
+    = get_improvement_type(id)->build_cost - shields_in_stock;
 
   if (building_has_effect(id, EFT_PROD_TO_GOLD)) {
     /* Can't buy capitalization. */
@@ -188,7 +194,7 @@ int impr_buy_gold_cost(Impr_type_id id, 
 ****************************************************************************/
 int impr_sell_gold(Impr_type_id id)
 {
-  return improvement_types[id].build_cost;
+  return get_improvement_type(id)->build_cost;
 }
 
 /**************************************************************************
@@ -206,7 +212,7 @@ Returns B_LAST if none match.
 Impr_type_id find_improvement_by_name(const char *s)
 {
   impr_type_iterate(i) {
-    if (strcmp(improvement_types[i].name, s)==0)
+    if (strcmp(get_improvement_name(i), s)==0)
       return i;
   } impr_type_iterate_end;
 
@@ -221,7 +227,7 @@ Impr_type_id find_improvement_by_name(co
 Impr_type_id find_improvement_by_name_orig(const char *s)
 {
   impr_type_iterate(i) {
-    if (mystrcasecmp(improvement_types[i].name_orig, s) == 0) {
+    if (mystrcasecmp(get_improvement_type(i)->name_orig, s) == 0) {
       return i;
     }
   } impr_type_iterate_end;
@@ -235,7 +241,7 @@ Impr_type_id find_improvement_by_name_or
 bool impr_flag(Impr_type_id id, enum impr_flag_id flag)
 {
   assert(flag >= 0 && flag < IF_LAST);
-  return TEST_BIT(improvement_types[id].flags, flag);
+  return TEST_BIT(get_improvement_type(id)->flags, flag);
 }
 
 /**************************************************************************
@@ -398,7 +404,7 @@ bool can_player_eventually_build_improve
 **************************************************************************/
 bool is_great_wonder(Impr_type_id id)
 {
-  return (improvement_types[id].genus == IG_GREAT_WONDER);
+  return (get_improvement_type(id)->genus == IG_GREAT_WONDER);
 }
 
 /**************************************************************************
@@ -406,7 +412,7 @@ bool is_great_wonder(Impr_type_id id)
 **************************************************************************/
 bool is_small_wonder(Impr_type_id id)
 {
-  return (improvement_types[id].genus == IG_SMALL_WONDER);
+  return (get_improvement_type(id)->genus == IG_SMALL_WONDER);
 }
 
 /**************************************************************************
@@ -414,7 +420,7 @@ bool is_small_wonder(Impr_type_id id)
 **************************************************************************/
 bool is_improvement(Impr_type_id id)
 {
-  return (improvement_types[id].genus == IG_IMPROVEMENT);
+  return (get_improvement_type(id)->genus == IG_IMPROVEMENT);
 }
 
 /**************************************************************************
Index: common/improvement.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/improvement.h,v
retrieving revision 1.51
diff -p -u -r1.51 improvement.h
--- common/improvement.h        22 Jul 2005 16:18:05 -0000      1.51
+++ common/improvement.h        23 Jul 2005 00:10:02 -0000
@@ -79,8 +79,6 @@ struct impr_type {
 };
 
 
-extern struct impr_type improvement_types[B_LAST];
-
 /* impr genus id/string converters */
 enum impr_genus_id impr_genus_from_str(const char *s);
 
Index: server/cityhand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/cityhand.c,v
retrieving revision 1.149
diff -p -u -r1.149 cityhand.c
--- server/cityhand.c   22 Jul 2005 16:18:06 -0000      1.149
+++ server/cityhand.c   23 Jul 2005 00:10:03 -0000
@@ -233,7 +233,7 @@ void really_handle_city_buy(struct playe
     assert(!pcity->is_building_unit);
     notify_player_ex(pplayer, pcity->tile, E_NOEVENT,
                      _("You don't buy %s!"),
-                    improvement_types[pcity->currently_building].name);
+                    get_improvement_name(pcity->currently_building));
     return;
   }
 
Index: server/citytools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/citytools.c,v
retrieving revision 1.337
diff -p -u -r1.337 citytools.c
--- server/citytools.c  22 Jul 2005 16:18:06 -0000      1.337
+++ server/citytools.c  23 Jul 2005 00:10:03 -0000
@@ -1818,7 +1818,7 @@ void change_build_target(struct player *
   if (is_unit)
     name = unit_types[pcity->currently_building].name;
   else
-    name = improvement_types[pcity->currently_building].name;
+    name = get_improvement_name(pcity->currently_building);
 
   switch (event) {
     case E_WORKLIST: source = _(" from the worklist"); break;
Index: server/cityturn.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/cityturn.c,v
retrieving revision 1.324
diff -p -u -r1.324 cityturn.c
--- server/cityturn.c   22 Jul 2005 16:18:06 -0000      1.324
+++ server/cityturn.c   23 Jul 2005 00:10:04 -0000
@@ -336,7 +336,7 @@ void send_city_turn_notifications(struct
                         E_CITY_GRAN_THROTTLE,
                         _("Suggest throttling growth in %s to use %s "
                           "(being built) more effectively."), pcity->name,
-                        improvement_types[pcity->currently_building].name);
+                      get_improvement_name(pcity->currently_building));
       }
     }
 
@@ -470,7 +470,7 @@ static void city_increase_size(struct ci
       notify_player_ex(powner, pcity->tile, E_CITY_AQ_BUILDING,
                       _("%s needs %s (being built) "
                         "to grow any further."), pcity->name,
-                      improvement_types[pcity->currently_building].name);
+                      get_improvement_name(pcity->currently_building));
     } else {
       notify_player_ex(powner, pcity->tile, E_CITY_AQUEDUCT,
                       _("%s needs an improvement to grow any further."),
@@ -901,7 +901,8 @@ static Impr_type_id building_upgrades_to
   if (!can_build_improvement_direct(pcity, check)) {
     return -1;
   }
-  while (improvement_exists(check = improvement_types[check].replaced_by)) {
+  while (improvement_exists(check
+                           = get_improvement_type(check)->replaced_by)) {
     if (can_build_improvement_direct(pcity, check)) {
       latest_ok = check;
     }
@@ -1272,7 +1273,7 @@ static void pay_for_buildings(struct pla
        notify_player_ex(pplayer, pcity->tile, E_IMP_AUCTIONED,
                         _("Can't afford to maintain %s in %s, "
                           "building sold!"),
-                        improvement_types[i].name, pcity->name);
+                        get_improvement_name(i), pcity->name);
        do_sell_building(pplayer, pcity, i);
        city_refresh(pcity);
       } else
@@ -1416,7 +1417,7 @@ static void define_orig_production_value
          pcity->name,
          pcity->changed_from_is_unit ?
          get_unit_type(pcity->changed_from_id)->name :
-           improvement_types[pcity->changed_from_id].name,
+         get_improvement_name(pcity->changed_from_id),
          pcity->before_change_shields
          );
 }
Index: server/diplomats.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/diplomats.c,v
retrieving revision 1.88
diff -p -u -r1.88 diplomats.c
--- server/diplomats.c  22 Jul 2005 16:18:06 -0000      1.88
+++ server/diplomats.c  23 Jul 2005 00:10:04 -0000
@@ -900,7 +900,7 @@ void diplomat_sabotage(struct player *pp
       } else {
        notify_player_ex(pplayer, pcity->tile, E_MY_DIPLOMAT_FAILED,
                         _("You cannot sabotage a %s!"),
-                        improvement_types[improvement].name);
+                        get_improvement_name(improvement));
        diplomat_charge_movement (pdiplomat, pcity->tile);
        send_unit_info (pplayer, pdiplomat);
        freelog (LOG_DEBUG, "sabotage: disallowed target improvement: %d (%s)",
Index: server/ruleset.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/ruleset.c,v
retrieving revision 1.275
diff -p -u -r1.275 ruleset.c
--- server/ruleset.c    22 Jul 2005 16:18:06 -0000      1.275
+++ server/ruleset.c    23 Jul 2005 00:10:05 -0000
@@ -1294,9 +1294,10 @@ static void load_building_names(struct s
 
   impr_type_iterate(i) {
     char *name = secfile_lookup_str(file, "%s.name", sec[i]);
+    struct impr_type *b = get_improvement_type(i);
 
-    name_strlcpy(improvement_types[i].name_orig, name);
-    improvement_types[i].name = improvement_types[i].name_orig;
+    name_strlcpy(b->name_orig, name);
+    b->name = b->name_orig;
   } impr_type_iterate_end;
 
   ruleset_cache_init();
@@ -1311,7 +1312,6 @@ static void load_ruleset_buildings(struc
 {
   char **sec, *item;
   int i, nval;
-  struct impr_type *b;
   const char *filename = secfile_filename(file);
 
   (void) check_ruleset_capabilities(file, "+1.10.1", filename);
@@ -1320,8 +1320,7 @@ static void load_ruleset_buildings(struc
 
   for (i = 0; i < nval; i++) {
     struct requirement_vector *reqs = lookup_req_list(file, sec[i], "reqs");
-
-    b = &improvement_types[i];
+    struct impr_type *b = get_improvement_type(i);
 
     item = secfile_lookup_str(file, "%s.genus", sec[i]);
     b->genus = impr_genus_from_str(item);
@@ -1370,7 +1369,8 @@ static void load_ruleset_buildings(struc
 
   /* Some more consistency checking: */
   impr_type_iterate(i) {
-    b = &improvement_types[i];
+    struct impr_type *b = get_improvement_type(i);
+
     if (improvement_exists(i)) {
       if (b->obsolete_by != A_LAST
          && (b->obsolete_by == A_NONE || !tech_exists(b->obsolete_by))) {
@@ -2741,7 +2741,7 @@ static void send_ruleset_techs(struct co
 static void send_ruleset_buildings(struct conn_list *dest)
 {
   impr_type_iterate(i) {
-    struct impr_type *b = &improvement_types[i];
+    struct impr_type *b = get_improvement_type(i);
     struct packet_ruleset_building packet;
     int j;
 
Index: server/techtools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/techtools.c,v
retrieving revision 1.16
diff -p -u -r1.16 techtools.c
--- server/techtools.c  3 Jul 2005 06:53:18 -0000       1.16
+++ server/techtools.c  23 Jul 2005 00:10:05 -0000
@@ -300,9 +300,9 @@ void found_new_tech(struct player *plr, 
 
     /* Alert the owners of any wonders that have been made obsolete */
     impr_type_iterate(id) {
-      if (is_great_wonder(id) && great_wonder_was_built(id) &&
-         improvement_types[id].obsolete_by == tech_found &&
-         (pcity = find_city_from_great_wonder(id))) {
+      if (is_great_wonder(id) && great_wonder_was_built(id)
+         && get_improvement_type(id)->obsolete_by == tech_found
+         && (pcity = find_city_from_great_wonder(id))) {
        notify_player_ex(city_owner(pcity), NULL, E_WONDER_OBSOLETE,
                         _("Discovery of %s OBSOLETES %s in %s!"), 
                         get_tech_name(city_owner(pcity), tech_found),