From 14e95c9675c8fb2ea3307e54d11ba029e20c0caa Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno@eressea.de>
Date: Sat, 13 Mar 2021 15:38:16 +0100
Subject: [PATCH] refactoring building_stage.construction

every building_stage has a construction.
no need to malloc it separately.
optimize some more allocations.
replace some memset and calloc calls.
---
 src/battle.test.c          |  7 +++----
 src/economy.c              |  2 +-
 src/exparse.c              | 33 ++++++++++++++++++---------------
 src/jsonconf.c             | 23 ++++++++++-------------
 src/jsonconf.test.c        | 13 ++++---------
 src/kernel/build.c         |  8 ++++----
 src/kernel/build.test.c    |  6 ++----
 src/kernel/building.c      |  5 ++---
 src/kernel/building.h      |  6 ++++--
 src/kernel/building.test.c | 24 ++++++++++--------------
 src/tests.c                | 15 ++++++---------
 11 files changed, 64 insertions(+), 78 deletions(-)

diff --git a/src/battle.test.c b/src/battle.test.c
index 41ae94db6..e9be1e93a 100644
--- a/src/battle.test.c
+++ b/src/battle.test.c
@@ -191,15 +191,14 @@ static building_type * setup_castle(void) {
 
     btype = test_create_buildingtype("castle");
     assert(btype->stages);
-    assert(btype->stages->construction);
 
     btype->flags |= BTF_FORTIFICATION;
-    cons = btype->stages->construction;
+    cons = &btype->stages->construction;
     cons->maxsize = 5;
     btype->stages->next = calloc(1, sizeof(building_stage));
-    cons = calloc(1, sizeof(construction));
+    assert(btype->stages->next);
+    cons = &btype->stages->next->construction;
     cons->maxsize = -1;
-    btype->stages->next->construction = cons;
     return btype;
 }
 
diff --git a/src/economy.c b/src/economy.c
index fff2f76b0..6ed5eda1e 100644
--- a/src/economy.c
+++ b/src/economy.c
@@ -1044,7 +1044,7 @@ int make_cmd(unit * u, struct order *ord)
         if (pl && fval(pl, PFL_NOBUILD)) {
             cmistake(u, ord, 275, MSG_PRODUCE);
         }
-        else if (btype->stages && btype->stages->construction) {
+        else if (btype->stages) {
             int id = getid();
             build_building(u, btype, id, m, ord);
         }
diff --git a/src/exparse.c b/src/exparse.c
index e5cca964c..b9d81a259 100644
--- a/src/exparse.c
+++ b/src/exparse.c
@@ -1276,16 +1276,13 @@ static void start_buildings(parseinfo *pi, const XML_Char *el, const XML_Char **
         }
         else if (xml_strequal(el, "requirement")) {
             assert(stage);
-            assert(stage->construction);
             handle_requirement(pi, el, attr);
         }
         else if (xml_strequal(el, "construction")) {
             assert(stage == NULL);
             stage = (building_stage *)calloc(1, sizeof(building_stage));
             if (!stage) abort();
-            stage->construction = calloc(1, sizeof(construction));
-            if (!stage->construction) abort();
-            parse_construction(stage->construction, pi, el, attr);
+            parse_construction(&stage->construction, pi, el, attr);
         }
         else if (xml_strequal(el, "maintenance")) {
             assert(!btype->maintenance);
@@ -1396,9 +1393,10 @@ static void end_weapon(parseinfo *pi, const XML_Char *el) {
     else if (xml_strequal(el, "modifier")) {
         if (nwmods > 0) {
             weapon_type *wtype = rtype->wtype;
-            wtype->modifiers = (weapon_mod *)calloc(nwmods + 1, sizeof(weapon_mod));
+            wtype->modifiers = malloc((1 + (size_t)nwmods) * sizeof(weapon_mod));
             if (!wtype->modifiers) abort();
             memcpy(wtype->modifiers, wmods, sizeof(weapon_mod) * nwmods);
+            wtype->modifiers[nwmods].value = 0;
             nwmods = 0;
         }
     }
@@ -1408,18 +1406,20 @@ static void end_resources(parseinfo *pi, const XML_Char *el) {
     resource_type *rtype = (resource_type *)pi->object;
     if (xml_strequal(el, "resource")) {
         if (nrmods > 0) {
-            rtype->modifiers = (resource_mod *)calloc(nrmods + 1, sizeof(resource_mod));
+            rtype->modifiers = malloc((1 + (size_t)nrmods) * sizeof(resource_mod));
             if (!rtype->modifiers) abort();
             memcpy(rtype->modifiers, rmods, sizeof(resource_mod) * nrmods);
+            rtype->modifiers[nrmods].type = RMT_END;
             nrmods = 0;
         }
     }
     else if (xml_strequal(el, "construction")) {
         if (nreqs > 0) {
             construction *con = rtype->itype->construction;
-            con->materials = (requirement *)calloc(nreqs + 1, sizeof(requirement));
+            con->materials = malloc((1 + (size_t)nreqs) * sizeof(requirement));
             if (!con->materials) abort();
             memcpy(con->materials, reqs, sizeof(requirement) * nreqs);
+            con->materials[nreqs].number = 0;
             nreqs = 0;
         }
     }
@@ -1455,17 +1455,19 @@ static void end_ships(parseinfo *pi, const XML_Char *el) {
         assert(stype->construction);
         if (nreqs > 0) {
             construction *con = stype->construction;
-            con->materials = (requirement *) calloc(nreqs + 1, sizeof(requirement));
+            con->materials = malloc((1 + (size_t)nreqs) * sizeof(requirement));
             if (!con->materials) abort();
             memcpy(con->materials, reqs, sizeof(requirement) * nreqs);
+            con->materials[nreqs].number = 0;
             nreqs = 0;
         }
     }
     else if (xml_strequal(el, "ship")) {
         if (ncoasts > 0) {
-            stype->coasts = (terrain_type **) calloc(ncoasts + 1, sizeof(terrain_type *));
+            stype->coasts = malloc((1 + (size_t)ncoasts) * sizeof(terrain_type *));
             if (!stype->coasts) abort();
             memcpy(stype->coasts, coasts, sizeof(terrain_type *) * ncoasts);
+            stype->coasts[ncoasts] = NULL;
             ncoasts = 0;
         }
         pi->object = NULL;
@@ -1484,10 +1486,11 @@ static void end_buildings(parseinfo *pi, const XML_Char *el) {
         assert(btype);
         if (stage) {
             if (nreqs > 0) {
-                construction *con = stage->construction;
-                con->materials = (requirement *)calloc(nreqs + 1, sizeof(requirement));
+                construction *con = &stage->construction;
+                con->materials = malloc((1 + (size_t)nreqs) * sizeof(requirement));
                 if (!con->materials) abort();
                 memcpy(con->materials, reqs, sizeof(requirement) * nreqs);
+                con->materials[nreqs].number = 0;
                 nreqs = 0;
             }
             if (stage_ptr == NULL) {
@@ -1503,17 +1506,17 @@ static void end_buildings(parseinfo *pi, const XML_Char *el) {
     else if (xml_strequal(el, "building")) {
         stage_ptr = NULL;
         if (nupkeep > 0) {
-            btype->maintenance = malloc((nupkeep + 1) * sizeof(maintenance));
+            btype->maintenance = malloc((1 + (size_t)nupkeep) * sizeof(maintenance));
             if (!btype->maintenance) abort();
             memcpy(btype->maintenance, upkeep, sizeof(maintenance) * nupkeep);
-            memset(btype->maintenance + nupkeep, 0, sizeof(maintenance));
+            btype->maintenance[nupkeep].number = 0;
             nupkeep = 0;
         }
         if (nrmods > 0) {
-            btype->modifiers = malloc((nrmods + 1) * sizeof(resource_mod));
+            btype->modifiers = malloc((1 + (size_t)nrmods) * sizeof(resource_mod));
             if (!btype->modifiers) abort();
             memcpy(btype->modifiers, rmods, sizeof(resource_mod) * nrmods);
-            memset(btype->modifiers + nrmods, 0, sizeof(resource_mod));
+            btype->modifiers[nrmods].type = RMT_END;
             nrmods = 0;
         }
         pi->object = NULL;
diff --git a/src/jsonconf.c b/src/jsonconf.c
index 42d61c7da..b54f6b670 100644
--- a/src/jsonconf.c
+++ b/src/jsonconf.c
@@ -155,16 +155,13 @@ static void json_maintenance(cJSON *json, maintenance **mtp) {
     }
 }
 
-static void json_construction(cJSON *json, construction **consp) {
+static void json_construction(cJSON *json, construction *cons) {
     cJSON *child;
-    construction * cons;
     
     if (json->type != cJSON_Object) {
         log_error("construction %s is not a json object: %d", json->string, json->type);
         return;
     }
-    cons = (construction *)calloc(1, sizeof(construction));
-    if (!cons) abort();
     for (child = json->child; child; child = child->next) {
         switch (child->type) {
         case cJSON_Object:
@@ -187,7 +184,6 @@ static void json_construction(cJSON *json, construction **consp) {
             log_error("construction %s contains unknown attribute %s", json->string, child->string);
         }
     }
-    *consp = cons;
 }
 
 static void json_terrain_production(cJSON *json, terrain_production *prod) {
@@ -345,9 +341,9 @@ static void json_stages(cJSON *json, building_type *bt) {
             stage = calloc(1, sizeof(building_stage));
             if (!stage) abort();
             json_stage(child, stage);
-            if (stage->construction->maxsize > 0) {
-                stage->construction->maxsize -= size;
-                size += stage->construction->maxsize;
+            if (stage->construction.maxsize > 0) {
+                stage->construction.maxsize -= size;
+                size += stage->construction.maxsize;
             }
             *sp = stage;
             sp = &stage->next;
@@ -447,19 +443,20 @@ static void json_ship(cJSON *json, ship_type *st) {
         return;
     }
     for (child = json->child; child; child = child->next) {
-        int i;
+        int i, n;
         switch (child->type) {
         case cJSON_Object:
             if (strcmp(child->string, "construction") == 0) {
-                json_construction(child, &st->construction);
+                st->construction = calloc(1, sizeof(construction));
+                json_construction(child, st->construction);
             }
             else {
                 log_error("ship %s contains unknown attribute %s", json->string, child->string);
             }
             break;
         case cJSON_Array:
-            st->coasts = (terrain_type **)
-                malloc(sizeof(terrain_type *) * (1 + cJSON_GetArraySize(child)));
+            n = cJSON_GetArraySize(child);
+            st->coasts = malloc(sizeof(terrain_type *) * (1 + (size_t)n));
             if (!st->coasts) abort();
             for (i = 0, iter = child->child; iter; iter = iter->next) {
                 if (iter->type == cJSON_String) {
@@ -469,7 +466,7 @@ static void json_ship(cJSON *json, ship_type *st) {
                     }
                 }
             }
-            st->coasts[i] = 0;
+            st->coasts[n] = 0;
             break;
         case cJSON_Number:
             if (strcmp(child->string, "range") == 0) {
diff --git a/src/jsonconf.test.c b/src/jsonconf.test.c
index 0f6db3e5e..edc5a6615 100644
--- a/src/jsonconf.test.c
+++ b/src/jsonconf.test.c
@@ -320,7 +320,6 @@ static void test_castles(CuTest *tc) {
     cJSON *json = cJSON_Parse(data);
     const building_type *bt;
     const building_stage *stage;
-    const construction *con;
 
     test_setup();
 
@@ -333,17 +332,14 @@ static void test_castles(CuTest *tc) {
     CuAssertPtrNotNull(tc, bt);
     CuAssertPtrNotNull(tc, stage = bt->stages);
     CuAssertStrEquals(tc, "site", stage->name);
-    CuAssertPtrNotNull(tc, con = stage->construction);
-    CuAssertIntEquals(tc, 2, con->maxsize);
+    CuAssertIntEquals(tc, 2, stage->construction.maxsize);
 
     CuAssertPtrNotNull(tc, stage = stage->next);
     CuAssertPtrEquals(tc, NULL, stage->name);
-    CuAssertPtrNotNull(tc, con = stage->construction);
-    CuAssertIntEquals(tc, 6, con->maxsize);
+    CuAssertIntEquals(tc, 6, stage->construction.maxsize);
 
     CuAssertPtrNotNull(tc, stage = stage->next);
-    CuAssertPtrNotNull(tc, con = stage->construction);
-    CuAssertIntEquals(tc, -1, con->maxsize);
+    CuAssertIntEquals(tc, -1, stage->construction.maxsize);
 
     CuAssertPtrEquals(tc, NULL, stage->next);
 
@@ -425,8 +421,7 @@ static void test_buildings(CuTest * tc)
 
     CuAssertPtrNotNull(tc, bt->stages);
     CuAssertPtrEquals(tc, NULL, bt->stages->next);
-    CuAssertPtrNotNull(tc, bt->stages->construction);
-    CuAssertPtrNotNull(tc, con = bt->stages->construction);
+    con = &bt->stages->construction;
     CuAssertPtrNotNull(tc, con->materials);
     CuAssertIntEquals(tc, 2, con->materials[0].number);
     CuAssertPtrEquals(tc, (void *)get_resourcetype(R_STONE), (void *)con->materials[0].rtype);
diff --git a/src/kernel/build.c b/src/kernel/build.c
index f5debccb8..6cc04d26a 100644
--- a/src/kernel/build.c
+++ b/src/kernel/build.c
@@ -201,7 +201,7 @@ int destroy_cmd(unit * u, struct order *ord)
             }
             ADDMSG(&u->faction->msgs, msg_message("destroy", "building unit", b, u));
             for (stage = b->type->stages; stage; stage = stage->next) {
-                size = recycle(u, stage->construction, size);
+                size = recycle(u, &stage->construction, size);
             }
             remove_building(&r->buildings, b);
         }
@@ -667,7 +667,7 @@ static int build_failure(unit *u, order *ord, const building_type *btype, int wa
         break;
     case ENOMATERIALS:
         ADDMSG(&u->faction->msgs, msg_materials_required(u, ord,
-            btype->stages->construction, want));
+            &btype->stages->construction, want));
         break;
     case ELOWSKILL:
     case ENEEDSKILL:
@@ -684,7 +684,7 @@ static int build_stages(unit *u, const building_type *btype, int built, int n, i
     int made = 0;
 
     for (stage = btype->stages; stage; stage = stage->next) {
-        const construction * con = stage->construction;
+        const construction * con = &stage->construction;
         if (con->maxsize < 0 || con->maxsize > built) {
             int err, want = INT_MAX;
             if (n < INT_MAX) {
@@ -740,7 +740,7 @@ build_building(unit * u, const building_type * btype, int id, int want, order *
     int skills, basesk;         /* number of skill points remainig */
 
     assert(u->number);
-    assert(btype->stages && btype->stages->construction);
+    assert(btype->stages);
 
     basesk = effskill(u, SK_BUILDING, NULL);
     skills = build_skill(u, basesk, 0);
diff --git a/src/kernel/build.test.c b/src/kernel/build.test.c
index aba3dcf74..8c8acaa79 100644
--- a/src/kernel/build.test.c
+++ b/src/kernel/build.test.c
@@ -66,8 +66,7 @@ static building_type *setup_castle(item_type *it_stone) {
     btype = test_create_buildingtype("castle");
     stage = btype->stages = calloc(1, sizeof(building_stage));
     if (!stage) abort();
-    cons = stage->construction = calloc(1, sizeof(construction));
-    if (!cons) abort();
+    cons = &stage->construction;
     cons->materials = calloc(2, sizeof(requirement));
     if (!cons->materials) abort();
     cons->materials[0].number = 1;
@@ -78,8 +77,7 @@ static building_type *setup_castle(item_type *it_stone) {
     cons->skill = SK_BUILDING;
     stage = stage->next = calloc(1, sizeof(building_stage));
     if (!stage) abort();
-    cons = stage->construction = calloc(1, sizeof(construction));
-    if (!cons) abort();
+    cons = &stage->construction;
     cons->materials = calloc(2, sizeof(requirement));
     if (!cons->materials) abort();
     cons->materials[0].number = 1;
diff --git a/src/kernel/building.c b/src/kernel/building.c
index 86c0d2b35..97e075f8f 100644
--- a/src/kernel/building.c
+++ b/src/kernel/building.c
@@ -102,7 +102,6 @@ static void free_buildingtype(void *ptr) {
     building_type *btype = (building_type *)ptr;
     while (btype->stages) {
         building_stage *next = btype->stages->next;
-        free_construction(btype->stages->construction);
         free(btype->stages->name);
         free(btype->stages);
         btype->stages = next;
@@ -190,7 +189,7 @@ const char *buildingtype(const building_type * btype, const building * b, int bs
             bsize = adjust_size(b, bsize);
         }
         for (stage = btype->stages; stage; stage = stage->next) {
-            bsize -= stage->construction->maxsize;
+            bsize -= stage->construction.maxsize;
             if (!stage->next || bsize <0) {
                 return stage->name;
             }
@@ -529,7 +528,7 @@ int bt_effsize(const building_type * btype, const building * b, int bsize)
         int n = 0;
         const building_stage *stage = btype->stages;
         do {
-            const construction *con = stage->construction;
+            const construction *con = &stage->construction;
             if (con->maxsize < 0) {
                 break;
             }
diff --git a/src/kernel/building.h b/src/kernel/building.h
index 3ebb52ec2..ffef979aa 100644
--- a/src/kernel/building.h
+++ b/src/kernel/building.h
@@ -1,7 +1,9 @@
 #ifndef H_KRNL_BUILDING
 #define H_KRNL_BUILDING
 
-#include <kernel/types.h>
+#include "types.h"
+#include "build.h"
+
 #include <util/resolve.h>
 #include <util/variant.h>
 
@@ -40,7 +42,7 @@ extern "C" {
 
     typedef struct building_stage {
         /* construction of this building stage: */
-        struct construction *construction;  
+        struct construction construction;  
         /* building stage name: */
         char * name;
         /* next stage, if upgradable: */
diff --git a/src/kernel/building.test.c b/src/kernel/building.test.c
index edbdd2e50..59b85e702 100644
--- a/src/kernel/building.test.c
+++ b/src/kernel/building.test.c
@@ -446,16 +446,13 @@ static void test_wage(CuTest *tc) {
     rc_elf->maintenance = 13;
     btype = test_create_buildingtype("castle");
     stage = btype->stages;
-    stage->construction->maxsize = 2; /* site */
+    stage->construction.maxsize = 2; /* site */
     stage = stage->next = calloc(1, sizeof(struct building_stage));
-    stage->construction = calloc(1, sizeof(struct construction));
-    stage->construction->maxsize = 8; /* tradepost */
+    stage->construction.maxsize = 8; /* tradepost */
     stage = stage->next = calloc(1, sizeof(struct building_stage));
-    stage->construction = calloc(1, sizeof(struct construction));
-    stage->construction->maxsize = 40; /* fortification */
+    stage->construction.maxsize = 40; /* fortification */
     stage = stage->next = calloc(1, sizeof(struct building_stage));
-    stage->construction = calloc(1, sizeof(struct construction));
-    stage->construction->maxsize = 200; /* fortification */
+    stage->construction.maxsize = 200; /* tower */
     r = test_create_plain(0, 0);
     CuAssertIntEquals(tc, 10, wage(r, rc_elf));
     CuAssertIntEquals(tc, 10, wage(r, rc_orc));
@@ -563,11 +560,11 @@ static void test_cmp_current_owner(CuTest *tc) {
     config_set("rules.region_owners", "1");
     r = test_create_region(0, 0, NULL);
     btype = test_create_buildingtype("watch");
-    btype->stages->construction->maxsize = 1;
+    btype->stages->construction.maxsize = 1;
     btype->taxes = 200;
     b1 = test_create_building(r, btype);
     btype = test_create_buildingtype("castle");
-    btype->stages->construction->maxsize = 1;
+    btype->stages->construction.maxsize = 1;
     btype->taxes = 100;
     b2 = test_create_building(r, btype);
     b1->size = 1;
@@ -595,18 +592,18 @@ static void test_building_effsize(CuTest *tc) {
     test_setup();
     btype = test_create_buildingtype("castle");
     stage = btype->stages;
-    assert(stage && stage->construction);
-    cons = stage->construction;
+    assert(stage);
+    cons = &stage->construction;
     cons->maxsize = 5;
 
     stage->next = calloc(1, sizeof(building_stage));
     stage = stage->next;
-    cons = stage->construction = calloc(1, sizeof(construction));
+    cons = &stage->construction;
     cons->maxsize = 5;
 
     stage->next = calloc(1, sizeof(building_stage));
     stage = stage->next;
-    cons = stage->construction = calloc(1, sizeof(construction));
+    cons = &stage->construction;
     cons->maxsize = -1;
 
     b = test_create_building(test_create_region(0,0,0), btype);
@@ -651,7 +648,6 @@ static void test_buildingtype(CuTest *tc) {
     btype = test_create_buildingtype("hodor");
     CuAssertPtrNotNull(tc, btype->stages);
     CuAssertPtrEquals(tc, NULL, btype->stages->name);
-    CuAssertPtrNotNull(tc, btype->stages->construction);
     CuAssertStrEquals(tc, "hodor", buildingtype(btype, NULL, 1));
 
     btype->stages->name = str_strdup("castle");
diff --git a/src/tests.c b/src/tests.c
index 270b34e21..ac9e8c0f4 100644
--- a/src/tests.c
+++ b/src/tests.c
@@ -401,17 +401,14 @@ building_type * test_create_buildingtype(const char * name)
     construction *con;
     building_type *btype = bt_get_or_create(name);
     if (btype->stages) {
-        con = btype->stages->construction;
+        con = &btype->stages->construction;
     } else {
         btype->stages = calloc(1, sizeof(building_stage));
-        con = (construction *)calloc(1, sizeof(construction));
-        if (con) {
-            con->skill = SK_BUILDING;
-            con->maxsize = -1;
-            con->minskill = 1;
-            con->reqsize = 1;
-            btype->stages->construction = con;
-        }
+        con = &btype->stages->construction;
+        con->skill = SK_BUILDING;
+        con->maxsize = -1;
+        con->minskill = 1;
+        con->reqsize = 1;
     }
     if (con && !con->materials) {
         con->materials = (requirement *)calloc(2, sizeof(requirement));