refactor: extract read_/write_building.

add a failing test for conversion of bad names.

src/kernel/save.c

@@ -1,4 +1,4 @@
-/*
+/*
 Copyright (c) 1998-2015, Enno Rehling <enno@eressea.de>
 Katja Zedel <katze@felidae.kn-bremen.de
 Christian Schlittchen <corwin@amber.kn-bremen.de>
@@ -1625,17 +1625,60 @@ int readgame(const char *filename, bool backup)
     return n;
 }
 
+void write_building(gamedata *data, const building *b)
+{
+    storage *store = data->store;
+
+    write_building_reference(b, store);
+    WRITE_STR(store, b->name);
+    WRITE_STR(store, b->display ? b->display : "");
+    WRITE_INT(store, b->size);
+    WRITE_TOK(store, b->type->_name);
+    write_attribs(store, b->attribs, b);
+}
+
+struct building *read_building(gamedata *data) {
+    char name[DISPLAYSIZE];
+	building *b;
+    storage * store = data->store;
+
+    b = (building *)calloc(1, sizeof(building));
+    READ_INT(store, &b->no);
+    bhash(b);
+    READ_STR(store, name, sizeof(name));
+    b->name = _strdup(name);
+    if (lomem) {
+        READ_STR(store, NULL, 0);
+    }
+    else {
+        READ_STR(store, name, sizeof(name));
+        b->display = _strdup(name);
+    }
+    READ_INT(store, &b->size);
+    READ_STR(store, name, sizeof(name));
+    b->type = bt_find(name);
+    read_attribs(data, &b->attribs, b);
+
+    // repairs, bug 2221:
+    if (b->type->maxsize>0 && b->size>b->type->maxsize) {
+        log_error("building too big: %s (%s size %d of %d), fixing.", buildingname(b), b->type->_name, b->size, b->type->maxsize);
+        b->size = b->type->maxsize;
+    }
+	return b;
+}
+
 int read_game(gamedata *data) {
     char name[DISPLAYSIZE];
     int n, p, nread;
     faction *f, **fp;
     region *r;
-    building *b, **bp;
+    building **bp;
     ship **shp;
     unit *u;
     int rmax = maxregions;
-    const struct building_type *bt_lighthouse = bt_find("lighthouse");
     storage * store = data->store;
+    const struct building_type *bt_lighthouse = bt_find("lighthouse");
+
     if (data->version >= SAVEGAMEID_VERSION) {
         int gameid;
 
@@ -1705,34 +1748,12 @@ int read_game(gamedata *data) {
         bp = &r->buildings;
 
         while (--p >= 0) {
-            b = (building *)calloc(1, sizeof(building));
+            building *b = *bp = read_building(data);
-            READ_INT(store, &b->no);
-            *bp = b;
-            bp = &b->next;
-            bhash(b);
-            READ_STR(store, name, sizeof(name));
-            b->name = _strdup(name);
-            if (lomem) {
-                READ_STR(store, NULL, 0);
-            }
-            else {
-                READ_STR(store, name, sizeof(name));
-                b->display = _strdup(name);
-            }
-            READ_INT(store, &b->size);
-            READ_STR(store, name, sizeof(name));
-            b->type = bt_find(name);
             b->region = r;
-            read_attribs(data, &b->attribs, b);
             if (b->type == bt_lighthouse) {
                 r->flags |= RF_LIGHTHOUSE;
             }
-
+            bp = &b->next;
-            // repairs, bug 2221:
-            if (b->type->maxsize>0 && b->size>b->type->maxsize) {
-                log_error("building too big: %s (%s size %d of %d), fixing.", buildingname(b), b->type->_name, b->size, b->type->maxsize);
-                b->size = b->type->maxsize;
-            }
         }
         /* Schiffe */
 
@@ -1979,14 +2000,7 @@ int write_game(gamedata *data) {
         WRITE_INT(store, listlen(r->buildings));
         WRITE_SECTION(store);
         for (b = r->buildings; b; b = b->next) {
-            write_building_reference(b, store);
+            write_building(data, b);
-            WRITE_STR(store, b->name);
-            WRITE_STR(store, b->display ? b->display : "");
-            WRITE_INT(store, b->size);
-            WRITE_TOK(store, b->type->_name);
-            WRITE_SECTION(store);
-            write_attribs(store, b->attribs, b);
-            WRITE_SECTION(store);
         }
 
         WRITE_INT(store, listlen(r->ships));

src/kernel/save.h

@@ -57,6 +57,9 @@ extern "C" {
 
     void write_unit(struct gamedata *data, const struct unit *u);
     struct unit *read_unit(struct gamedata *data);
+    
+    void write_building(struct gamedata *data, const struct building *b);
+    struct building *read_building(struct gamedata *data);
 
     int a_readint(struct attrib *a, void *owner, struct gamedata *);
     void a_writeint(const struct attrib *a, const void *owner,

src/kernel/save.test.c

@@ -6,6 +6,7 @@
 
 #include "save.h"
 #include "version.h"
+#include "building.h"
 #include "unit.h"
 #include "group.h"
 #include "ally.h"
@@ -27,6 +28,7 @@
 #include <tests.h>
 
 #include <stdio.h>
+#include <string.h>
 #include <errno.h>
 
 static void test_readwrite_data(CuTest * tc)
@@ -79,6 +81,39 @@ static void test_readwrite_unit(CuTest * tc)
     test_cleanup();
 }
 
+static void test_readwrite_building(CuTest * tc)
+{
+    gamedata data;
+    storage store;
+    struct building *b;
+    struct region *r;
+
+    test_setup();
+    r = test_create_region(0, 0, 0);
+    b = test_create_building(r, 0);
+    free(b->name);
+    b->name = strdup("  Hodor  ");
+    CuAssertStrEquals(tc, "  Hodor  ", b->name);
+    mstream_init(&data.strm);
+    gamedata_init(&data, &store, RELEASE_VERSION);
+    write_building(&data, b);
+    
+    data.strm.api->rewind(data.strm.handle);
+    free_gamedata();
+    r = test_create_region(0, 0, 0);
+    gamedata_init(&data, &store, RELEASE_VERSION);
+    b = read_building(&data);
+    CuAssertPtrNotNull(tc, b);
+    CuAssertStrEquals(tc, "Hodor", b->name);
+    CuAssertPtrEquals(tc, 0, b->region);
+    b->region = r;
+    r->buildings = b;
+
+    mstream_done(&data.strm);
+    gamedata_done(&data);
+    test_cleanup();
+}
+
 static void test_readwrite_attrib(CuTest *tc) {
     gamedata data;
     storage store;
@@ -328,6 +363,7 @@ CuSuite *get_save_suite(void)
     SUITE_ADD_TEST(suite, test_readwrite_attrib);
     SUITE_ADD_TEST(suite, test_readwrite_data);
     SUITE_ADD_TEST(suite, test_readwrite_unit);
+    SUITE_ADD_TEST(suite, test_readwrite_building);
     SUITE_ADD_TEST(suite, test_readwrite_dead_faction_createunit);
     SUITE_ADD_TEST(suite, test_readwrite_dead_faction_changefaction);
     SUITE_ADD_TEST(suite, test_readwrite_dead_faction_regionowner);

src/tests.c

@@ -217,7 +217,9 @@ test_create_terrain(const char * name, unsigned int flags)
 
 building * test_create_building(region * r, const building_type * btype)
 {
-    building * b = new_building(btype ? btype : test_create_buildingtype("castle"), r, default_locale);
+    building * b;
+    assert(r);
+    b = new_building(btype ? btype : test_create_buildingtype("castle"), r, default_locale);
     b->size = b->type->maxsize > 0 ? b->type->maxsize : 1;
     return b;
 }