From f68fcf40a658671507ea044b41a4eb8fbabcfb22 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Tue, 8 Nov 2016 22:54:47 +0100 Subject: [PATCH 01/17] custom function to make "valid" names. remove any non-printable characters from a string. TODO: test with utf-8 values. TODO: remove leading/trainling whitespace. --- src/util/unicode.c | 32 ++++++++++++++++++++++++++++++++ src/util/unicode.h | 4 ++-- src/util/unicode.test.c | 12 ++++++++++++ 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/src/util/unicode.c b/src/util/unicode.c index f7ea2653b..dd8e03814 100644 --- a/src/util/unicode.c +++ b/src/util/unicode.c @@ -14,6 +14,7 @@ #include #include #include +#include #define B00000000 0x00 #define B10000000 0x80 @@ -31,6 +32,37 @@ #define B00000011 0x03 #define B00000001 0x01 +int unicode_utf8_mkname(utf8_t * op, size_t outlen, const utf8_t * ip) +{ + while (*ip) { + ucs4_t ucs = *ip; + size_t size = 1; + bool isp = false; +// bool iss = false; + if (ucs & 0x80) { + int ret = unicode_utf8_to_ucs4(&ucs, ip, &size); + if (ret !=0) { + return ret; + } + isp = iswprint(ucs); +// iss = iswspace(ucs); + } else { + isp = isprint(ucs); +// iss = isspace(ucs); + } + if (size > outlen) { + return ENOMEM; + } + if (isp) { + memcpy(op, ip, size); + op += size; + outlen -= size; + } + ip += size; + } + return 0; +} + int unicode_utf8_tolower(utf8_t * op, size_t outlen, const utf8_t * ip) { while (*ip) { diff --git a/src/util/unicode.h b/src/util/unicode.h index df68ade02..c817a04e3 100644 --- a/src/util/unicode.h +++ b/src/util/unicode.h @@ -41,8 +41,8 @@ extern "C" { int unicode_utf8_strcasecmp(const utf8_t * a, const utf8_t * b); int unicode_latin1_to_utf8(utf8_t * out, size_t * outlen, const char *in, size_t * inlen); - int unicode_utf8_tolower(utf8_t * out, size_t outlen, - const utf8_t * in); + int unicode_utf8_tolower(utf8_t *op, size_t outlen, const utf8_t *ip); + int unicode_utf8_mkname(utf8_t *op, size_t outlen, const utf8_t *ip); #ifdef __cplusplus } diff --git a/src/util/unicode.test.c b/src/util/unicode.test.c index 410e8e68a..3cfa0615c 100644 --- a/src/util/unicode.test.c +++ b/src/util/unicode.test.c @@ -5,6 +5,17 @@ #include #include +static void test_unicode_mkname(CuTest * tc) +{ + char buffer[32]; + CuAssertIntEquals(tc, 0, unicode_utf8_mkname(buffer, sizeof(buffer), "HeLlO\nW0Rld")); + CuAssertStrEquals(tc, "HeLlOW0Rld", buffer); + memset(buffer, 0, sizeof(buffer)); + buffer[5] = 'X'; + CuAssertIntEquals(tc, ENOMEM, unicode_utf8_mkname(buffer, 5, "HeLl\n W0Rld")); + CuAssertStrEquals(tc, "HeLl X", buffer); +} + static void test_unicode_tolower(CuTest * tc) { char buffer[32]; @@ -66,6 +77,7 @@ CuSuite *get_unicode_suite(void) { CuSuite *suite = CuSuiteNew(); SUITE_ADD_TEST(suite, test_unicode_tolower); + SUITE_ADD_TEST(suite, test_unicode_mkname); SUITE_ADD_TEST(suite, test_unicode_utf8_to_other); return suite; } From 7d79822aa40d626a6ccf5990de2ead7abb5fef0f Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Wed, 9 Nov 2016 11:54:34 +0100 Subject: [PATCH 02/17] fix missing nul-terminator. --- src/util/unicode.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/util/unicode.c b/src/util/unicode.c index dd8e03814..c4dfb40e2 100644 --- a/src/util/unicode.c +++ b/src/util/unicode.c @@ -60,6 +60,10 @@ int unicode_utf8_mkname(utf8_t * op, size_t outlen, const utf8_t * ip) } ip += size; } + if (outlen <= 0) { + return ENOMEM; + } + *op = 0; return 0; } From f03e8f3b62619519e397cabd411f30801febf521 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Wed, 9 Nov 2016 14:58:51 +0100 Subject: [PATCH 03/17] return an error code if the string had any characters removed. --- src/util/unicode.c | 7 +++++-- src/util/unicode.test.c | 5 ++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/util/unicode.c b/src/util/unicode.c index c4dfb40e2..6ae1a0676 100644 --- a/src/util/unicode.c +++ b/src/util/unicode.c @@ -34,13 +34,14 @@ int unicode_utf8_mkname(utf8_t * op, size_t outlen, const utf8_t * ip) { + int ret = 0; while (*ip) { ucs4_t ucs = *ip; size_t size = 1; bool isp = false; // bool iss = false; if (ucs & 0x80) { - int ret = unicode_utf8_to_ucs4(&ucs, ip, &size); + ret = unicode_utf8_to_ucs4(&ucs, ip, &size); if (ret !=0) { return ret; } @@ -57,6 +58,8 @@ int unicode_utf8_mkname(utf8_t * op, size_t outlen, const utf8_t * ip) memcpy(op, ip, size); op += size; outlen -= size; + } else { + ret = 1; } ip += size; } @@ -64,7 +67,7 @@ int unicode_utf8_mkname(utf8_t * op, size_t outlen, const utf8_t * ip) return ENOMEM; } *op = 0; - return 0; + return ret; } int unicode_utf8_tolower(utf8_t * op, size_t outlen, const utf8_t * ip) diff --git a/src/util/unicode.test.c b/src/util/unicode.test.c index 3cfa0615c..e70e03752 100644 --- a/src/util/unicode.test.c +++ b/src/util/unicode.test.c @@ -8,7 +8,10 @@ static void test_unicode_mkname(CuTest * tc) { char buffer[32]; - CuAssertIntEquals(tc, 0, unicode_utf8_mkname(buffer, sizeof(buffer), "HeLlO\nW0Rld")); + CuAssertIntEquals(tc, 0, unicode_utf8_mkname(buffer, sizeof(buffer), "HeLlO W0Rld")); + CuAssertStrEquals(tc, "HeLlO W0Rld", buffer); + memset(buffer, 0, sizeof(buffer)); + CuAssertIntEquals(tc, 1, unicode_utf8_mkname(buffer, sizeof(buffer), "HeLlO\nW0Rld")); CuAssertStrEquals(tc, "HeLlOW0Rld", buffer); memset(buffer, 0, sizeof(buffer)); buffer[5] = 'X'; From 7b412399b7cd6360bfa97a2d7add216359d20975 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Wed, 9 Nov 2016 22:03:46 +0100 Subject: [PATCH 04/17] delete leading spaces from names. --- src/util/unicode.c | 31 ++++++++++++++++++------------- src/util/unicode.test.c | 3 ++- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/src/util/unicode.c b/src/util/unicode.c index 6ae1a0676..98d9f2d07 100644 --- a/src/util/unicode.c +++ b/src/util/unicode.c @@ -35,22 +35,27 @@ int unicode_utf8_mkname(utf8_t * op, size_t outlen, const utf8_t * ip) { int ret = 0; + bool iss = true; while (*ip) { - ucs4_t ucs = *ip; size_t size = 1; bool isp = false; -// bool iss = false; - if (ucs & 0x80) { - ret = unicode_utf8_to_ucs4(&ucs, ip, &size); - if (ret !=0) { - return ret; - } - isp = iswprint(ucs); -// iss = iswspace(ucs); - } else { - isp = isprint(ucs); -// iss = isspace(ucs); - } + do { + ucs4_t ucs = *ip; + if (ucs & 0x80) { + ret = unicode_utf8_to_ucs4(&ucs, ip, &size); + if (ret !=0) { + return ret; + } + isp = iswprint(ucs); + iss &= !!iswspace(ucs); + } else { + isp = isprint(ucs); + iss &= !!isspace(ucs); + } + if (iss) { + ip += size; + } + } while (iss); if (size > outlen) { return ENOMEM; } diff --git a/src/util/unicode.test.c b/src/util/unicode.test.c index e70e03752..6d4f0fd67 100644 --- a/src/util/unicode.test.c +++ b/src/util/unicode.test.c @@ -8,9 +8,10 @@ static void test_unicode_mkname(CuTest * tc) { char buffer[32]; + CuAssertIntEquals(tc, 0, unicode_utf8_mkname(buffer, sizeof(buffer), " HeLlO W0Rld")); + CuAssertStrEquals(tc, "HeLlO W0Rld", buffer); CuAssertIntEquals(tc, 0, unicode_utf8_mkname(buffer, sizeof(buffer), "HeLlO W0Rld")); CuAssertStrEquals(tc, "HeLlO W0Rld", buffer); - memset(buffer, 0, sizeof(buffer)); CuAssertIntEquals(tc, 1, unicode_utf8_mkname(buffer, sizeof(buffer), "HeLlO\nW0Rld")); CuAssertStrEquals(tc, "HeLlOW0Rld", buffer); memset(buffer, 0, sizeof(buffer)); From 97e91fd8eb2c0edd208fceb8673bcc6c75e0f7e2 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Fri, 11 Nov 2016 00:30:49 +0100 Subject: [PATCH 05/17] fix bad names. trim leading whitespace and non-printable characters from existing unit, region and faction names/descriptions. --- src/kernel/save.c | 20 ++++++++++++- src/util/unicode.c | 62 ++++++++++++++++++++++++++++++----------- src/util/unicode.h | 1 + src/util/unicode.test.c | 22 +++++++++++++++ 4 files changed, 88 insertions(+), 17 deletions(-) diff --git a/src/kernel/save.c b/src/kernel/save.c index 93c4f7e4b..eae3ee6c4 100644 --- a/src/kernel/save.c +++ b/src/kernel/save.c @@ -69,6 +69,7 @@ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. #include #include #include +#include #include #include @@ -747,12 +748,18 @@ unit *read_unit(struct gamedata *data) } READ_STR(data->store, obuf, sizeof(obuf)); + if (unicode_utf8_trim(obuf)!=0) { + log_error("trim unit %s name to '%s'", itoa36(u->no), obuf); + }; u->_name = obuf[0] ? _strdup(obuf) : 0; if (lomem) { READ_STR(data->store, NULL, 0); } else { READ_STR(data->store, obuf, sizeof(obuf)); + if (unicode_utf8_trim(obuf)!=0) { + log_error("trim unit %s info to '%s'", itoa36(u->no), obuf); + }; u->display = obuf[0] ? _strdup(obuf) : 0; } READ_INT(data->store, &number); @@ -986,6 +993,9 @@ static region *readregion(struct gamedata *data, int x, int y) else { char info[DISPLAYSIZE]; READ_STR(data->store, info, sizeof(info)); + if (unicode_utf8_trim(info)!=0) { + log_error("trim region %d info to '%s'", uid, info); + }; region_setinfo(r, info); } @@ -1003,6 +1013,9 @@ static region *readregion(struct gamedata *data, int x, int y) if (fval(r->terrain, LAND_REGION)) { r->land = calloc(1, sizeof(land_region)); READ_STR(data->store, name, sizeof(name)); + if (unicode_utf8_trim(name)!=0) { + log_error("trim region %d name to '%s'", uid, name); + }; r->land->name = _strdup(name); } if (r->land) { @@ -1386,8 +1399,14 @@ faction *readfaction(struct gamedata * data) } READ_STR(data->store, name, sizeof(name)); + if (unicode_utf8_trim(name)!=0) { + log_error("trim faction %s name to '%s'", itoa36(f->no), name); + }; f->name = _strdup(name); READ_STR(data->store, name, sizeof(name)); + if (unicode_utf8_trim(name)!=0) { + log_error("trim faction %s banner to '%s'", itoa36(f->no), name); + }; f->banner = _strdup(name); log_debug(" - Lese Partei %s (%s)", f->name, factionid(f)); @@ -1686,7 +1705,6 @@ int read_game(gamedata *data) { bp = &r->buildings; while (--p >= 0) { - b = (building *)calloc(1, sizeof(building)); READ_INT(store, &b->no); *bp = b; diff --git a/src/util/unicode.c b/src/util/unicode.c index 98d9f2d07..15b914fac 100644 --- a/src/util/unicode.c +++ b/src/util/unicode.c @@ -32,6 +32,36 @@ #define B00000011 0x03 #define B00000001 0x01 +int unicode_utf8_trim(utf8_t *buf) +{ + int result = 0; + utf8_t *op = buf, *ip = buf; + while (*ip) { + ucs4_t ucs = *ip; + size_t size = 1; + if (ucs & 0x80) { + int ret = unicode_utf8_to_ucs4(&ucs, ip, &size); + if (ret != 0) { + return ret; + } + } + if (op == buf && iswspace(ucs)) { + ++result; + } + else if (iswprint(ucs)) { + if (op != ip) { + memcpy(op, ip, size); + } + op += size; + } else { + ++result; + } + ip += size; + } + *op = '\0'; + return result; +} + int unicode_utf8_mkname(utf8_t * op, size_t outlen, const utf8_t * ip) { int ret = 0; @@ -40,22 +70,22 @@ int unicode_utf8_mkname(utf8_t * op, size_t outlen, const utf8_t * ip) size_t size = 1; bool isp = false; do { - ucs4_t ucs = *ip; - if (ucs & 0x80) { - ret = unicode_utf8_to_ucs4(&ucs, ip, &size); - if (ret !=0) { - return ret; - } - isp = iswprint(ucs); - iss &= !!iswspace(ucs); - } else { - isp = isprint(ucs); - iss &= !!isspace(ucs); - } - if (iss) { - ip += size; - } - } while (iss); + ucs4_t ucs = *ip; + if (ucs & 0x80) { + ret = unicode_utf8_to_ucs4(&ucs, ip, &size); + if (ret !=0) { + return ret; + } + isp = iswprint(ucs); + iss &= !!iswspace(ucs); + } else { + isp = isprint(ucs); + iss &= !!isspace(ucs); + } + if (iss) { + ip += size; + } + } while (iss); if (size > outlen) { return ENOMEM; } diff --git a/src/util/unicode.h b/src/util/unicode.h index c817a04e3..3231f1e94 100644 --- a/src/util/unicode.h +++ b/src/util/unicode.h @@ -43,6 +43,7 @@ extern "C" { const char *in, size_t * inlen); int unicode_utf8_tolower(utf8_t *op, size_t outlen, const utf8_t *ip); int unicode_utf8_mkname(utf8_t *op, size_t outlen, const utf8_t *ip); + int unicode_utf8_trim(utf8_t *ip); #ifdef __cplusplus } diff --git a/src/util/unicode.test.c b/src/util/unicode.test.c index 6d4f0fd67..db3fb0ccf 100644 --- a/src/util/unicode.test.c +++ b/src/util/unicode.test.c @@ -5,6 +5,27 @@ #include #include +static void test_unicode_trim(CuTest * tc) +{ + char buffer[32]; + + strcpy(buffer, "Hello Word"); + CuAssertIntEquals(tc, 0, unicode_utf8_trim(buffer)); + CuAssertStrEquals(tc, "Hello Word", buffer); + + strcpy(buffer, "Hello Word\n"); + CuAssertIntEquals(tc, 1, unicode_utf8_trim(buffer)); + CuAssertStrEquals(tc, "Hello Word", buffer); + + strcpy(buffer, " Hello Word\t\n"); + CuAssertIntEquals(tc, 4, unicode_utf8_trim(buffer)); + CuAssertStrEquals(tc, "Hello Word", buffer); + + strcpy(buffer, " \t Hello Word"); + CuAssertIntEquals(tc, 3, unicode_utf8_trim(buffer)); + CuAssertStrEquals(tc, "Hello Word", buffer); +} + static void test_unicode_mkname(CuTest * tc) { char buffer[32]; @@ -82,6 +103,7 @@ CuSuite *get_unicode_suite(void) CuSuite *suite = CuSuiteNew(); SUITE_ADD_TEST(suite, test_unicode_tolower); SUITE_ADD_TEST(suite, test_unicode_mkname); + SUITE_ADD_TEST(suite, test_unicode_trim); SUITE_ADD_TEST(suite, test_unicode_utf8_to_other); return suite; } From e0add2275ffea20eec0c7af3369eb51ccd2bf498 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Fri, 11 Nov 2016 01:34:59 +0100 Subject: [PATCH 06/17] also remove trailing spaces. --- src/util/unicode.c | 25 +++++++++++++++---------- src/util/unicode.test.c | 4 ++++ 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/util/unicode.c b/src/util/unicode.c index 15b914fac..0e6aedb9f 100644 --- a/src/util/unicode.c +++ b/src/util/unicode.c @@ -34,8 +34,8 @@ int unicode_utf8_trim(utf8_t *buf) { - int result = 0; - utf8_t *op = buf, *ip = buf; + int result = 0, ts = 0; + utf8_t *op = buf, *ip = buf, *lc = buf; while (*ip) { ucs4_t ucs = *ip; size_t size = 1; @@ -46,20 +46,25 @@ int unicode_utf8_trim(utf8_t *buf) } } if (op == buf && iswspace(ucs)) { - ++result; - } + ++result; + } else if (iswprint(ucs)) { - if (op != ip) { - memcpy(op, ip, size); + if (op != ip) { + memcpy(op, ip, size); } op += size; + if (iswspace(ucs)) ++ts; + else { + lc = op; + ts = 0; + } } else { - ++result; - } + ++result; + } ip += size; } - *op = '\0'; - return result; + *lc = '\0'; + return result + ts; } int unicode_utf8_mkname(utf8_t * op, size_t outlen, const utf8_t * ip) diff --git a/src/util/unicode.test.c b/src/util/unicode.test.c index db3fb0ccf..167538a3f 100644 --- a/src/util/unicode.test.c +++ b/src/util/unicode.test.c @@ -13,6 +13,10 @@ static void test_unicode_trim(CuTest * tc) CuAssertIntEquals(tc, 0, unicode_utf8_trim(buffer)); CuAssertStrEquals(tc, "Hello Word", buffer); + strcpy(buffer, " Hello Word "); + CuAssertIntEquals(tc, 4, unicode_utf8_trim(buffer)); + CuAssertStrEquals(tc, "Hello Word", buffer); + strcpy(buffer, "Hello Word\n"); CuAssertIntEquals(tc, 1, unicode_utf8_trim(buffer)); CuAssertStrEquals(tc, "Hello Word", buffer); From ff09defa695b9d1f0f7ecb3471fc5714f8e7d7ec Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Fri, 11 Nov 2016 01:59:43 +0100 Subject: [PATCH 07/17] prevent badly naming units/factions/regions. --- src/laws.c | 14 +++++++------- src/laws.test.c | 26 ++++++++++++++++++++++++-- 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/src/laws.c b/src/laws.c index 9e53a465f..de87afeb1 100644 --- a/src/laws.c +++ b/src/laws.c @@ -79,17 +79,17 @@ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. #include #include #include +#include #include #include -#include #include #include #include -#include -#include +#include #include +#include #include /* libc includes */ #include @@ -1627,6 +1627,7 @@ bool renamed_building(const building * b) static int rename_cmd(unit * u, order * ord, char **s, const char *s2) { + char name[NAMESIZE]; assert(s2); if (!s2[0]) { cmistake(u, ord, 84, MSG_EVENT); @@ -1635,12 +1636,11 @@ static int rename_cmd(unit * u, order * ord, char **s, const char *s2) /* TODO: Validate to make sure people don't have illegal characters in * names, phishing-style? () come to mind. */ + strlcpy(name, s2, sizeof(name)); + unicode_utf8_trim(name); free(*s); - *s = _strdup(s2); - if (strlen(s2) >= NAMESIZE) { - (*s)[NAMESIZE] = 0; - } + *s = _strdup(name); return 0; } diff --git a/src/laws.test.c b/src/laws.test.c index 65f48012d..0f3566714 100644 --- a/src/laws.test.c +++ b/src/laws.test.c @@ -1024,7 +1024,7 @@ static void test_ally_cmd_errors(CuTest *tc) { int fid; order *ord; - test_cleanup(); + test_setup(); u = test_create_unit(test_create_faction(0), test_create_region(0, 0, 0)); fid = u->faction->no + 1; CuAssertPtrEquals(tc, 0, findfaction(fid)); @@ -1037,12 +1037,33 @@ static void test_ally_cmd_errors(CuTest *tc) { test_cleanup(); } +static void test_name_cmd(CuTest *tc) { + unit *u; + faction *f; + order *ord; + + test_setup(); + u = test_create_unit(f = test_create_faction(0), test_create_region(0, 0, 0)); + + ord = create_order(K_NAME, f->locale, "%s ' Ho\tdor '", LOC(f->locale, parameters[P_UNIT])); + name_cmd(u, ord); + CuAssertStrEquals(tc, "Hodor", u->_name); + free_order(ord); + + ord = create_order(K_NAME, f->locale, "%s ' Ho\tdor '", LOC(f->locale, parameters[P_FACTION])); + name_cmd(u, ord); + CuAssertStrEquals(tc, "Hodor", f->name); + free_order(ord); + + test_cleanup(); +} + static void test_ally_cmd(CuTest *tc) { unit *u; faction * f; order *ord; - test_cleanup(); + test_setup(); u = test_create_unit(test_create_faction(0), test_create_region(0, 0, 0)); f = test_create_faction(0); @@ -1444,6 +1465,7 @@ CuSuite *get_laws_suite(void) CuSuite *suite = CuSuiteNew(); SUITE_ADD_TEST(suite, test_nmr_warnings); SUITE_ADD_TEST(suite, test_ally_cmd); + SUITE_ADD_TEST(suite, test_name_cmd); SUITE_ADD_TEST(suite, test_ally_cmd_errors); SUITE_ADD_TEST(suite, test_long_order_normal); SUITE_ADD_TEST(suite, test_long_order_none); From bc44caa17f58d17040d0550a4ff544e0f30d973d Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Fri, 11 Nov 2016 02:07:11 +0100 Subject: [PATCH 08/17] backfill tests for renaming regions. --- src/laws.test.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/laws.test.c b/src/laws.test.c index 0f3566714..922cfb676 100644 --- a/src/laws.test.c +++ b/src/laws.test.c @@ -1055,6 +1055,14 @@ static void test_name_cmd(CuTest *tc) { CuAssertStrEquals(tc, "Hodor", f->name); free_order(ord); + ord = create_order(K_NAME, f->locale, "%s ' Ho\tdor '", LOC(f->locale, parameters[P_REGION])); + name_cmd(u, ord); + CuAssertPtrNotNull(tc, test_find_messagetype(f->msgs, "error145")); + u->building = test_create_building(u->region, 0); + name_cmd(u, ord); + CuAssertStrEquals(tc, "Hodor", u->region->land->name); + free_order(ord); + test_cleanup(); } From dea10209835354077876bb76e8e987c6ce7481ff Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Fri, 11 Nov 2016 11:09:57 +0100 Subject: [PATCH 09/17] log a warning, not an error, when fixing bad save data. --- src/kernel/save.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/kernel/save.c b/src/kernel/save.c index eae3ee6c4..7c5091d41 100644 --- a/src/kernel/save.c +++ b/src/kernel/save.c @@ -749,7 +749,7 @@ unit *read_unit(struct gamedata *data) READ_STR(data->store, obuf, sizeof(obuf)); if (unicode_utf8_trim(obuf)!=0) { - log_error("trim unit %s name to '%s'", itoa36(u->no), obuf); + log_warning("trim unit %s name to '%s'", itoa36(u->no), obuf); }; u->_name = obuf[0] ? _strdup(obuf) : 0; if (lomem) { @@ -758,7 +758,7 @@ unit *read_unit(struct gamedata *data) else { READ_STR(data->store, obuf, sizeof(obuf)); if (unicode_utf8_trim(obuf)!=0) { - log_error("trim unit %s info to '%s'", itoa36(u->no), obuf); + log_warning("trim unit %s info to '%s'", itoa36(u->no), obuf); }; u->display = obuf[0] ? _strdup(obuf) : 0; } @@ -994,7 +994,7 @@ static region *readregion(struct gamedata *data, int x, int y) char info[DISPLAYSIZE]; READ_STR(data->store, info, sizeof(info)); if (unicode_utf8_trim(info)!=0) { - log_error("trim region %d info to '%s'", uid, info); + log_warning("trim region %d info to '%s'", uid, info); }; region_setinfo(r, info); } @@ -1014,7 +1014,7 @@ static region *readregion(struct gamedata *data, int x, int y) r->land = calloc(1, sizeof(land_region)); READ_STR(data->store, name, sizeof(name)); if (unicode_utf8_trim(name)!=0) { - log_error("trim region %d name to '%s'", uid, name); + log_warning("trim region %d name to '%s'", uid, name); }; r->land->name = _strdup(name); } @@ -1400,12 +1400,12 @@ faction *readfaction(struct gamedata * data) READ_STR(data->store, name, sizeof(name)); if (unicode_utf8_trim(name)!=0) { - log_error("trim faction %s name to '%s'", itoa36(f->no), name); + log_warning("trim faction %s name to '%s'", itoa36(f->no), name); }; f->name = _strdup(name); READ_STR(data->store, name, sizeof(name)); if (unicode_utf8_trim(name)!=0) { - log_error("trim faction %s banner to '%s'", itoa36(f->no), name); + log_warning("trim faction %s banner to '%s'", itoa36(f->no), name); }; f->banner = _strdup(name); From 64f80274a09dadafa27c19c00a4a4fb6c15c66d7 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Fri, 11 Nov 2016 14:00:04 +0100 Subject: [PATCH 10/17] test that ship and building cannot be named poorly. --- src/laws.test.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/laws.test.c b/src/laws.test.c index 922cfb676..69e9f5e04 100644 --- a/src/laws.test.c +++ b/src/laws.test.c @@ -1055,11 +1055,20 @@ static void test_name_cmd(CuTest *tc) { CuAssertStrEquals(tc, "Hodor", f->name); free_order(ord); - ord = create_order(K_NAME, f->locale, "%s ' Ho\tdor '", LOC(f->locale, parameters[P_REGION])); + ord = create_order(K_NAME, f->locale, "%s ' Ho\tdor '", LOC(f->locale, parameters[P_SHIP])); + u->ship = test_create_ship(u->region, 0); name_cmd(u, ord); - CuAssertPtrNotNull(tc, test_find_messagetype(f->msgs, "error145")); + CuAssertStrEquals(tc, "Hodor", u->ship->name); + free_order(ord); + + ord = create_order(K_NAME, f->locale, "%s ' Ho\tdor '", LOC(f->locale, parameters[P_BUILDING])); u->building = test_create_building(u->region, 0); name_cmd(u, ord); + CuAssertStrEquals(tc, "Hodor", u->building->name); + free_order(ord); + + ord = create_order(K_NAME, f->locale, "%s ' Ho\tdor '", LOC(f->locale, parameters[P_REGION])); + name_cmd(u, ord); CuAssertStrEquals(tc, "Hodor", u->region->land->name); free_order(ord); From a47da05f515f425f301eba5eed7e8e756cda2494 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Fri, 11 Nov 2016 15:41:34 +0100 Subject: [PATCH 11/17] test unit names get fixed during read. --- src/kernel/save.test.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/kernel/save.test.c b/src/kernel/save.test.c index 00b5ff96c..2b1f0464d 100644 --- a/src/kernel/save.test.c +++ b/src/kernel/save.test.c @@ -55,7 +55,8 @@ static void test_readwrite_unit(CuTest * tc) f = test_create_faction(0); fno = f->no; u = test_create_unit(f, r); - + unit_setname(u, " Hodor "); + CuAssertStrEquals(tc, " Hodor ", u->_name); mstream_init(&data.strm); gamedata_init(&data, &store, RELEASE_VERSION); write_unit(&data, u); @@ -69,6 +70,7 @@ static void test_readwrite_unit(CuTest * tc) u = read_unit(&data); CuAssertPtrNotNull(tc, u); CuAssertPtrEquals(tc, f, u->faction); + CuAssertStrEquals(tc, "Hodor", u->_name); CuAssertPtrEquals(tc, 0, u->region); mstream_done(&data.strm); From 599c4228524e1c22f6fa04fc37bda86e6dd71212 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Fri, 11 Nov 2016 21:41:25 +0100 Subject: [PATCH 12/17] refactor: extract read_/write_building. add a failing test for conversion of bad names. --- src/kernel/save.c | 84 ++++++++++++++++++++++++------------------ src/kernel/save.h | 3 ++ src/kernel/save.test.c | 36 ++++++++++++++++++ src/tests.c | 4 +- 4 files changed, 91 insertions(+), 36 deletions(-) diff --git a/src/kernel/save.c b/src/kernel/save.c index 7c5091d41..cdedbb525 100644 --- a/src/kernel/save.c +++ b/src/kernel/save.c @@ -1,4 +1,4 @@ -/* +/* Copyright (c) 1998-2015, Enno Rehling Katja Zedel @@ -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)); - 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); + building *b = *bp = read_building(data); b->region = r; - read_attribs(data, &b->attribs, b); if (b->type == bt_lighthouse) { r->flags |= RF_LIGHTHOUSE; } - - // 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; - } + bp = &b->next; } /* 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_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_building(data, b); } WRITE_INT(store, listlen(r->ships)); diff --git a/src/kernel/save.h b/src/kernel/save.h index 3cd443926..6854bad3a 100644 --- a/src/kernel/save.h +++ b/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, diff --git a/src/kernel/save.test.c b/src/kernel/save.test.c index 2b1f0464d..9bb544462 100644 --- a/src/kernel/save.test.c +++ b/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 #include +#include #include 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); diff --git a/src/tests.c b/src/tests.c index 177ad50fc..05855563a 100644 --- a/src/tests.c +++ b/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; } From 110e87916d2ee85776e96d32cacfed48d013ffd3 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Fri, 11 Nov 2016 21:46:56 +0100 Subject: [PATCH 13/17] fix bad ship names in save file. --- src/kernel/save.c | 10 ++++++++-- src/kernel/save.test.c | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/kernel/save.c b/src/kernel/save.c index cdedbb525..2abd0b93f 100644 --- a/src/kernel/save.c +++ b/src/kernel/save.c @@ -750,7 +750,7 @@ unit *read_unit(struct gamedata *data) READ_STR(data->store, obuf, sizeof(obuf)); if (unicode_utf8_trim(obuf)!=0) { log_warning("trim unit %s name to '%s'", itoa36(u->no), obuf); - }; + } u->_name = obuf[0] ? _strdup(obuf) : 0; if (lomem) { READ_STR(data->store, NULL, 0); @@ -759,7 +759,7 @@ unit *read_unit(struct gamedata *data) READ_STR(data->store, obuf, sizeof(obuf)); if (unicode_utf8_trim(obuf)!=0) { log_warning("trim unit %s info to '%s'", itoa36(u->no), obuf); - }; + } u->display = obuf[0] ? _strdup(obuf) : 0; } READ_INT(data->store, &number); @@ -1646,12 +1646,18 @@ struct building *read_building(gamedata *data) { READ_INT(store, &b->no); bhash(b); READ_STR(store, name, sizeof(name)); + if (unicode_utf8_trim(name)!=0) { + log_warning("trim building %s name to '%s'", itoa36(b->no), name); + } b->name = _strdup(name); if (lomem) { READ_STR(store, NULL, 0); } else { READ_STR(store, name, sizeof(name)); + if (unicode_utf8_trim(name)!=0) { + log_warning("trim building %s info to '%s'", itoa36(b->no), name); + } b->display = _strdup(name); } READ_INT(store, &b->size); diff --git a/src/kernel/save.test.c b/src/kernel/save.test.c index 9bb544462..af7c4c1f4 100644 --- a/src/kernel/save.test.c +++ b/src/kernel/save.test.c @@ -92,7 +92,7 @@ static void test_readwrite_building(CuTest * tc) r = test_create_region(0, 0, 0); b = test_create_building(r, 0); free(b->name); - b->name = strdup(" Hodor "); + b->name = _strdup(" Hodor "); CuAssertStrEquals(tc, " Hodor ", b->name); mstream_init(&data.strm); gamedata_init(&data, &store, RELEASE_VERSION); From 5032d44af46d16a9f11403392dbcdbcb42559a4e Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Fri, 11 Nov 2016 22:25:56 +0100 Subject: [PATCH 14/17] refactor: extract read_/write_ship functions. add test: bad names are cleaned up (failing). --- src/kernel/save.c | 116 +++++++++++++++++++++++------------------ src/kernel/save.h | 5 ++ src/kernel/save.test.c | 39 +++++++++++++- 3 files changed, 108 insertions(+), 52 deletions(-) diff --git a/src/kernel/save.c b/src/kernel/save.c index 2abd0b93f..528a71808 100644 --- a/src/kernel/save.c +++ b/src/kernel/save.c @@ -1673,9 +1673,68 @@ struct building *read_building(gamedata *data) { return b; } -int read_game(gamedata *data) { +void write_ship(gamedata *data, const ship *sh) +{ + storage *store = data->store; + write_ship_reference(sh, store); + WRITE_STR(store, (const char *)sh->name); + WRITE_STR(store, sh->display ? (const char *)sh->display : ""); + WRITE_TOK(store, sh->type->_name); + WRITE_INT(store, sh->size); + WRITE_INT(store, sh->damage); + WRITE_INT(store, sh->flags & SFL_SAVEMASK); + assert((sh->type->flags & SFL_NOCOAST) == 0 || sh->coast == NODIRECTION); + WRITE_INT(store, sh->coast); + write_attribs(store, sh->attribs, sh); +} + +ship *read_ship(struct gamedata *data) +{ char name[DISPLAYSIZE]; - int n, p, nread; + ship *sh; + int n; + storage *store = data->store; + + sh = (ship *)calloc(1, sizeof(ship)); + READ_INT(store, &sh->no); + shash(sh); + READ_STR(store, name, sizeof(name)); + sh->name = _strdup(name); + if (lomem) { + READ_STR(store, NULL, 0); + } + else { + READ_STR(store, name, sizeof(name)); + sh->display = _strdup(name); + } + READ_STR(store, name, sizeof(name)); + sh->type = st_find(name); + if (sh->type == NULL) { + /* old datafiles */ + sh->type = st_find((const char *)LOC(default_locale, name)); + } + assert(sh->type || !"ship_type not registered!"); + + READ_INT(store, &sh->size); + READ_INT(store, &sh->damage); + if (data->version >= FOSS_VERSION) { + READ_INT(store, &sh->flags); + } + + /* Attribute rekursiv einlesen */ + + READ_INT(store, &n); + sh->coast = (direction_t)n; + if (sh->type->flags & SFL_NOCOAST) { + sh->coast = NODIRECTION; + } + read_attribs(data, &sh->attribs, sh); + return sh; +} + + +int read_game(gamedata *data) { + int p, nread; faction *f, **fp; region *r; building **bp; @@ -1755,10 +1814,10 @@ int read_game(gamedata *data) { while (--p >= 0) { building *b = *bp = read_building(data); - b->region = r; if (b->type == bt_lighthouse) { r->flags |= RF_LIGHTHOUSE; } + b->region = r; bp = &b->next; } /* Schiffe */ @@ -1767,43 +1826,9 @@ int read_game(gamedata *data) { shp = &r->ships; while (--p >= 0) { - ship *sh = (ship *)calloc(1, sizeof(ship)); + ship *sh = *shp = read_ship(data); sh->region = r; - READ_INT(store, &sh->no); - *shp = sh; shp = &sh->next; - shash(sh); - READ_STR(store, name, sizeof(name)); - sh->name = _strdup(name); - if (lomem) { - READ_STR(store, NULL, 0); - } - else { - READ_STR(store, name, sizeof(name)); - sh->display = _strdup(name); - } - READ_STR(store, name, sizeof(name)); - sh->type = st_find(name); - if (sh->type == NULL) { - /* old datafiles */ - sh->type = st_find((const char *)LOC(default_locale, name)); - } - assert(sh->type || !"ship_type not registered!"); - - READ_INT(store, &sh->size); - READ_INT(store, &sh->damage); - if (data->version >= FOSS_VERSION) { - READ_INT(store, &sh->flags); - } - - /* Attribute rekursiv einlesen */ - - READ_INT(store, &n); - sh->coast = (direction_t)n; - if (sh->type->flags & SFL_NOCOAST) { - sh->coast = NODIRECTION; - } - read_attribs(data, &sh->attribs, sh); } *shp = 0; @@ -2006,6 +2031,7 @@ int write_game(gamedata *data) { WRITE_INT(store, listlen(r->buildings)); WRITE_SECTION(store); for (b = r->buildings; b; b = b->next) { + assert(b->region == r); write_building(data, b); } @@ -2013,23 +2039,13 @@ int write_game(gamedata *data) { WRITE_SECTION(store); for (sh = r->ships; sh; sh = sh->next) { assert(sh->region == r); - write_ship_reference(sh, store); - WRITE_STR(store, (const char *)sh->name); - WRITE_STR(store, sh->display ? (const char *)sh->display : ""); - WRITE_TOK(store, sh->type->_name); - WRITE_INT(store, sh->size); - WRITE_INT(store, sh->damage); - WRITE_INT(store, sh->flags & SFL_SAVEMASK); - assert((sh->type->flags & SFL_NOCOAST) == 0 || sh->coast == NODIRECTION); - WRITE_INT(store, sh->coast); - WRITE_SECTION(store); - write_attribs(store, sh->attribs, sh); - WRITE_SECTION(store); + write_ship(data, sh); } WRITE_INT(store, listlen(r->units)); WRITE_SECTION(store); for (u = r->units; u; u = u->next) { + assert(u->region == r); write_unit(data, u); } } diff --git a/src/kernel/save.h b/src/kernel/save.h index 6854bad3a..d74017f62 100644 --- a/src/kernel/save.h +++ b/src/kernel/save.h @@ -30,6 +30,8 @@ extern "C" { struct spell; struct spellbook; struct unit; + struct building; + struct ship; struct gamedata; #define MAX_INPUT_SIZE DISPLAYSIZE*2 @@ -61,6 +63,9 @@ extern "C" { void write_building(struct gamedata *data, const struct building *b); struct building *read_building(struct gamedata *data); + void write_ship(struct gamedata *data, const struct ship *sh); + struct ship *read_ship(struct gamedata *data); + int a_readint(struct attrib *a, void *owner, struct gamedata *); void a_writeint(const struct attrib *a, const void *owner, struct storage *store); diff --git a/src/kernel/save.test.c b/src/kernel/save.test.c index af7c4c1f4..4f13851cb 100644 --- a/src/kernel/save.test.c +++ b/src/kernel/save.test.c @@ -7,6 +7,7 @@ #include "save.h" #include "version.h" #include "building.h" +#include "ship.h" #include "unit.h" #include "group.h" #include "ally.h" @@ -85,8 +86,8 @@ static void test_readwrite_building(CuTest * tc) { gamedata data; storage store; - struct building *b; - struct region *r; + building *b; + region *r; test_setup(); r = test_create_region(0, 0, 0); @@ -114,6 +115,39 @@ static void test_readwrite_building(CuTest * tc) test_cleanup(); } +static void test_readwrite_ship(CuTest * tc) +{ + gamedata data; + storage store; + ship *sh; + region *r; + + test_setup(); + r = test_create_region(0, 0, 0); + sh = test_create_ship(r, 0); + free(sh->name); + sh->name = _strdup(" Hodor "); + CuAssertStrEquals(tc, " Hodor ", sh->name); + mstream_init(&data.strm); + gamedata_init(&data, &store, RELEASE_VERSION); + write_ship(&data, sh); + + data.strm.api->rewind(data.strm.handle); + free_gamedata(); + r = test_create_region(0, 0, 0); + gamedata_init(&data, &store, RELEASE_VERSION); + sh = read_ship(&data); + CuAssertPtrNotNull(tc, sh); + CuAssertStrEquals(tc, "Hodor", sh->name); + CuAssertPtrEquals(tc, 0, sh->region); + sh->region = r; + r->ships = sh; + + mstream_done(&data.strm); + gamedata_done(&data); + test_cleanup(); +} + static void test_readwrite_attrib(CuTest *tc) { gamedata data; storage store; @@ -364,6 +398,7 @@ CuSuite *get_save_suite(void) 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_ship); 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); From 710811131f57ec36ea911ea83664c975ad881b43 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Fri, 11 Nov 2016 22:28:22 +0100 Subject: [PATCH 15/17] fix bad ship and building names and descriptions in save. --- src/kernel/save.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/kernel/save.c b/src/kernel/save.c index 528a71808..d063fc8f7 100644 --- a/src/kernel/save.c +++ b/src/kernel/save.c @@ -1699,12 +1699,18 @@ ship *read_ship(struct gamedata *data) READ_INT(store, &sh->no); shash(sh); READ_STR(store, name, sizeof(name)); + if (unicode_utf8_trim(name)!=0) { + log_warning("trim ship %s name to '%s'", itoa36(sh->no), name); + } sh->name = _strdup(name); if (lomem) { READ_STR(store, NULL, 0); } else { READ_STR(store, name, sizeof(name)); + if (unicode_utf8_trim(name)!=0) { + log_warning("trim ship %s info to '%s'", itoa36(sh->no), name); + } sh->display = _strdup(name); } READ_STR(store, name, sizeof(name)); From ec787743f00fecef6dc12b8d1531dfc00eedb555 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sat, 12 Nov 2016 00:47:25 +0100 Subject: [PATCH 16/17] extract read_faction and test that it repairs bad names --- src/kernel/save.c | 16 +++++++++------- src/kernel/save.h | 3 +++ src/kernel/save.test.c | 30 ++++++++++++++++++++++++++++++ 3 files changed, 42 insertions(+), 7 deletions(-) diff --git a/src/kernel/save.c b/src/kernel/save.c index d063fc8f7..96263feed 100644 --- a/src/kernel/save.c +++ b/src/kernel/save.c @@ -1348,7 +1348,7 @@ void _test_write_password(gamedata *data, const faction *f) { * This function requires no context, can be called in any state. The * faction may not already exist, however. */ -faction *readfaction(struct gamedata * data) +faction *read_faction(struct gamedata * data) { ally **sfp; int planes, n; @@ -1363,9 +1363,10 @@ faction *readfaction(struct gamedata * data) f->no = n; } else { - f->allies = NULL; /* mem leak */ - while (f->attribs) + f->allies = NULL; /* FIXME: mem leak */ + while (f->attribs) { a_remove(&f->attribs, f->attribs); + } } READ_INT(data->store, &f->subscription); @@ -1499,7 +1500,7 @@ faction *readfaction(struct gamedata * data) return f; } -void writefaction(struct gamedata *data, const faction * f) +void write_faction(struct gamedata *data, const faction * f) { ally *sf; ursprung *ur; @@ -1781,11 +1782,12 @@ int read_game(gamedata *data) { READ_INT(store, &nread); log_debug(" - Einzulesende Parteien: %d\n", nread); fp = &factions; - while (*fp) + while (*fp) { fp = &(*fp)->next; + } while (--nread >= 0) { - faction *f = readfaction(data); + faction *f = read_faction(data); *fp = f; fp = &f->next; @@ -2010,7 +2012,7 @@ int write_game(gamedata *data) { if (fval(f, FFL_NPC)) { clear_npc_orders(f); } - writefaction(data, f); + write_faction(data, f); WRITE_SECTION(store); } diff --git a/src/kernel/save.h b/src/kernel/save.h index d74017f62..7c28d6c43 100644 --- a/src/kernel/save.h +++ b/src/kernel/save.h @@ -60,6 +60,9 @@ extern "C" { void write_unit(struct gamedata *data, const struct unit *u); struct unit *read_unit(struct gamedata *data); + void write_faction(struct gamedata *data, const struct faction *f); + struct faction *read_faction(struct gamedata *data); + void write_building(struct gamedata *data, const struct building *b); struct building *read_building(struct gamedata *data); diff --git a/src/kernel/save.test.c b/src/kernel/save.test.c index 4f13851cb..298b9e21f 100644 --- a/src/kernel/save.test.c +++ b/src/kernel/save.test.c @@ -82,6 +82,35 @@ static void test_readwrite_unit(CuTest * tc) test_cleanup(); } +static void test_readwrite_faction(CuTest * tc) +{ + gamedata data; + storage store; + faction *f; + + test_setup(); + f = test_create_faction(0); + free(f->name); + f->name = _strdup(" Hodor "); + CuAssertStrEquals(tc, " Hodor ", f->name); + mstream_init(&data.strm); + gamedata_init(&data, &store, RELEASE_VERSION); + write_faction(&data, f); + + data.strm.api->rewind(data.strm.handle); + free_gamedata(); + gamedata_init(&data, &store, RELEASE_VERSION); + f = read_faction(&data); + CuAssertPtrNotNull(tc, f); + CuAssertStrEquals(tc, "Hodor", f->name); + CuAssertPtrEquals(tc, 0, f->units); + factions = f; + + mstream_done(&data.strm); + gamedata_done(&data); + test_cleanup(); +} + static void test_readwrite_building(CuTest * tc) { gamedata data; @@ -397,6 +426,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_faction); SUITE_ADD_TEST(suite, test_readwrite_building); SUITE_ADD_TEST(suite, test_readwrite_ship); SUITE_ADD_TEST(suite, test_readwrite_dead_faction_createunit); From 663ad17b5adaf249b347593d3b377e63ca2bd0b4 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sat, 12 Nov 2016 01:03:07 +0100 Subject: [PATCH 17/17] extract read_region and test that it fixes bad names. --- src/kernel/save.c | 45 ++++++++++++++++++++++++++---------------- src/kernel/save.h | 3 +++ src/kernel/save.test.c | 29 +++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 17 deletions(-) diff --git a/src/kernel/save.c b/src/kernel/save.c index 96263feed..327fa4f1a 100644 --- a/src/kernel/save.c +++ b/src/kernel/save.c @@ -1125,6 +1125,17 @@ static region *readregion(struct gamedata *data, int x, int y) return r; } +region *read_region(gamedata *data) +{ + storage *store = data->store; + region *r; + int x, y; + READ_INT(store, &x); + READ_INT(store, &y); + r = readregion(data, x, y); + return r; +} + void writeregion(struct gamedata *data, const region * r) { assert(r); @@ -1189,6 +1200,14 @@ void writeregion(struct gamedata *data, const region * r) WRITE_SECTION(data->store); } +void write_region(gamedata *data, const region *r) +{ + storage *store = data->store; + WRITE_INT(store, r->x); + WRITE_INT(store, r->y); + writeregion(data, r); +} + static ally **addally(const faction * f, ally ** sfp, int aid, int state) { struct faction *af = findfaction(aid); @@ -1344,10 +1363,6 @@ void _test_write_password(gamedata *data, const faction *f) { write_password(data, f); } -/** Reads a faction from a file. - * This function requires no context, can be called in any state. The - * faction may not already exist, however. - */ faction *read_faction(struct gamedata * data) { ally **sfp; @@ -1803,18 +1818,11 @@ int read_game(gamedata *data) { rmax = nread; } log_debug(" - Einzulesende Regionen: %d/%d\r", rmax, nread); + while (--nread >= 0) { unit **up; - int x, y; - READ_INT(store, &x); - READ_INT(store, &y); - if ((nread & 0x3FF) == 0) { /* das spart extrem Zeit */ - log_debug(" - Einzulesende Regionen: %d/%d * %d,%d \r", rmax, nread, x, y); - } - --rmax; - - r = readregion(data, x, y); + r = read_region(data); /* Burgen */ READ_INT(store, &p); @@ -1861,8 +1869,13 @@ int read_game(gamedata *data) { *up = u; up = &u->next; - update_interval(u->faction, u->region); + update_interval(u->faction, r); } + + if ((nread & 0x3FF) == 0) { /* das spart extrem Zeit */ + log_debug(" - Einzulesende Regionen: %d/%d * %d,%d \r", rmax, nread, r->x, r->y); + } + --rmax; } read_borders(data); @@ -2032,9 +2045,7 @@ int write_game(gamedata *data) { log_debug(" - Schreibe Regionen: %d", n); } WRITE_SECTION(store); - WRITE_INT(store, r->x); - WRITE_INT(store, r->y); - writeregion(data, r); + write_region(data, r); WRITE_INT(store, listlen(r->buildings)); WRITE_SECTION(store); diff --git a/src/kernel/save.h b/src/kernel/save.h index 7c28d6c43..ffc9c2891 100644 --- a/src/kernel/save.h +++ b/src/kernel/save.h @@ -63,6 +63,9 @@ extern "C" { void write_faction(struct gamedata *data, const struct faction *f); struct faction *read_faction(struct gamedata *data); + void write_region(struct gamedata *data, const struct region *r); + struct region *read_region(struct gamedata *data); + void write_building(struct gamedata *data, const struct building *b); struct building *read_building(struct gamedata *data); diff --git a/src/kernel/save.test.c b/src/kernel/save.test.c index 298b9e21f..b4d79f4f8 100644 --- a/src/kernel/save.test.c +++ b/src/kernel/save.test.c @@ -111,6 +111,34 @@ static void test_readwrite_faction(CuTest * tc) test_cleanup(); } +static void test_readwrite_region(CuTest * tc) +{ + gamedata data; + storage store; + region *r; + + test_setup(); + r = test_create_region(0, 0, 0); + free(r->land->name); + r->land->name = _strdup(" Hodor "); + CuAssertStrEquals(tc, " Hodor ", r->land->name); + mstream_init(&data.strm); + gamedata_init(&data, &store, RELEASE_VERSION); + write_region(&data, r); + + data.strm.api->rewind(data.strm.handle); + free_gamedata(); + gamedata_init(&data, &store, RELEASE_VERSION); + r = read_region(&data); + CuAssertPtrNotNull(tc, r); + CuAssertStrEquals(tc, "Hodor", r->land->name); + regions = r; + + mstream_done(&data.strm); + gamedata_done(&data); + test_cleanup(); +} + static void test_readwrite_building(CuTest * tc) { gamedata data; @@ -427,6 +455,7 @@ CuSuite *get_save_suite(void) SUITE_ADD_TEST(suite, test_readwrite_data); SUITE_ADD_TEST(suite, test_readwrite_unit); SUITE_ADD_TEST(suite, test_readwrite_faction); + SUITE_ADD_TEST(suite, test_readwrite_region); SUITE_ADD_TEST(suite, test_readwrite_building); SUITE_ADD_TEST(suite, test_readwrite_ship); SUITE_ADD_TEST(suite, test_readwrite_dead_faction_createunit);