Merge pull request #620 from ennorehling/develop

eliminate strncpy from the code, reduce snprintf calls
This commit is contained in:
Enno Rehling 2017-01-07 21:25:12 +01:00 committed by GitHub
commit a3cdf03ebd
21 changed files with 107 additions and 88 deletions

View File

@ -3,6 +3,7 @@
#include <platform.h>
#include <kernel/config.h>
#include <kernel/jsonconf.h>
#include <util/bsdstring.h>
#include <util/log.h>
#include <util/language.h>
#include <cJSON.h>
@ -45,7 +46,7 @@ int config_parse(const char *json)
if (xp >= ep) break;
}
xp = (ep > json + 10) ? ep - 10 : json;
strncpy(buffer, xp, sizeof(buffer));
strlcpy(buffer, xp, sizeof(buffer));
buffer[9] = 0;
log_error("json parse error in line %d, position %d, near `%s`\n", line, ep - lp, buffer);
}

View File

@ -58,34 +58,6 @@ static int tolua_spawn_undead(lua_State * L)
return 0;
}
static int fix_familiars(struct lua_State *L)
{
faction *f;
for (f = factions; f; f = f->next) {
unit *u;
for (u = f->units; u; u = u->nextF) {
struct sc_mage *mage = get_mage(u);
if (mage && is_familiar(u)) {
if (mage->spellbook && mage->magietyp == M_GRAY) {
equipment *eq;
char buffer[64];
spellbook_clear(mage->spellbook);
free(mage->spellbook);
mage->spellbook = 0;
_snprintf(buffer, sizeof(buffer), "%s_familiar", u_race(u)->_name);
eq = get_equipment(buffer);
if (eq) {
equip_unit_mask(u, eq, EQUIP_SPELLS);
}
}
}
}
}
return 0;
}
void bind_monsters(struct lua_State *L)
{
tolua_module(L, NULL, 0);
@ -95,7 +67,6 @@ void bind_monsters(struct lua_State *L)
tolua_function(L, TOLUA_CAST "plan_monsters", tolua_planmonsters);
tolua_function(L, TOLUA_CAST "spawn_undead", tolua_spawn_undead);
tolua_function(L, TOLUA_CAST "spawn_dragons", tolua_spawn_dragons);
tolua_function(L, TOLUA_CAST "fix_familiars", fix_familiars);
tolua_function(L, TOLUA_CAST "get_monsters", tolua_get_monsters);
}
tolua_endmodule(L);

View File

@ -100,7 +100,9 @@ static int tolua_storage_tostring(lua_State * L)
{
gamedata *data = (gamedata *)tolua_tousertype(L, 1, 0);
char name[64];
_snprintf(name, sizeof(name), "<gamedata %p ver=%d>", (void *)data, data->version);
// safe to use sprintf here, because:
// %p is at most 16 characters, %d 20, text is 16, comes to 53 with \0
sprintf(name, "<gamedata %p ver=%d>", (void *)data, data->version);
lua_pushstring(L, name);
return 1;
}

View File

@ -233,7 +233,6 @@ static void test_give_men_requires_contact(CuTest * tc) {
struct give env = { 0 };
message * msg;
order *ord;
char cmd[32];
test_setup_ex(tc);
env.f1 = test_create_faction(0);
@ -244,8 +243,7 @@ static void test_give_men_requires_contact(CuTest * tc) {
CuAssertIntEquals(tc, 1, env.dst->number);
CuAssertIntEquals(tc, 1, env.src->number);
_snprintf(cmd, sizeof(cmd), "%s ALLES PERSONEN", itoa36(env.dst->no));
ord = create_order(K_GIVE, env.f1->locale, cmd);
ord = create_order(K_GIVE, env.f1->locale, "%s ALLES PERSONEN", itoa36(env.dst->no));
test_clear_messages(env.f1);
give_cmd(env.src, ord);
CuAssertPtrEquals(tc, 0, test_find_messagetype(env.f1->msgs, "give_person"));
@ -307,7 +305,6 @@ static void test_give(CuTest * tc) {
static void test_give_cmd(CuTest * tc) {
struct give env = { 0 };
struct order *ord;
char cmd[32];
test_setup_ex(tc);
env.lang = test_create_locale();
@ -316,8 +313,7 @@ static void test_give_cmd(CuTest * tc) {
i_change(&env.src->items, env.itype, 10);
_snprintf(cmd, sizeof(cmd), "%s 5 %s", itoa36(env.dst->no), LOC(env.f1->locale, env.itype->rtype->_name));
ord = create_order(K_GIVE, env.f1->locale, cmd);
ord = create_order(K_GIVE, env.f1->locale, "%s 5 %s", itoa36(env.dst->no), LOC(env.f1->locale, env.itype->rtype->_name));
assert(ord);
give_cmd(env.src, ord);
CuAssertIntEquals(tc, 5, i_get(env.src->items, env.itype));
@ -330,7 +326,6 @@ static void test_give_cmd(CuTest * tc) {
static void test_give_herbs(CuTest * tc) {
struct give env = { 0 };
struct order *ord;
char cmd[32];
test_setup_ex(tc);
test_create_world();
@ -338,8 +333,7 @@ static void test_give_herbs(CuTest * tc) {
setup_give(&env);
i_change(&env.src->items, env.itype, 10);
_snprintf(cmd, sizeof(cmd), "%s %s", itoa36(env.dst->no), LOC(env.f1->locale, parameters[P_HERBS]));
ord = create_order(K_GIVE, env.f1->locale, cmd);
ord = create_order(K_GIVE, env.f1->locale, "%s %s", itoa36(env.dst->no), LOC(env.f1->locale, parameters[P_HERBS]));
assert(ord);
give_cmd(env.src, ord);

View File

@ -1167,7 +1167,7 @@ static void handlekey(state * st, int c)
region *first = (mr && mr->r && mr->r->next) ? mr->r->next : regions;
if (findmode == 'f') {
snprintf(sbuffer, sizeof(sbuffer), "find-faction: %s", locate);
slprintf(sbuffer, sizeof(sbuffer), "find-faction: %s", locate);
statusline(st->wnd_status->handle, sbuffer);
f = findfaction(atoi36(locate));
if (f == NULL) {

View File

@ -167,7 +167,7 @@ static int lua_callspell(castorder * co)
if (hashpos != NULL) {
ptrdiff_t len = hashpos - fname;
assert(len < (ptrdiff_t) sizeof(fbuf));
strncpy(fbuf, fname, len);
memcpy(fbuf, fname, len);
fbuf[len] = '\0';
fname = fbuf;
}

View File

@ -67,13 +67,13 @@ int json_export(stream * out, int flags) {
cJSON *json, *root = cJSON_CreateObject();
assert(out && out->api);
if (regions && (flags & EXPORT_REGIONS)) {
char id[32];
char id[32]; // TODO: static_assert(INT_MAX < 10^32)
region * r;
plane * p;
cJSON_AddItemToObject(root, "planes", json = cJSON_CreateObject());
for (p = planes; p; p = p->next) {
cJSON *data;
_snprintf(id, sizeof(id), "%d", p->id);
sprintf(id, "%d", p->id); // safe, unless int is bigger than 64 bit
cJSON_AddItemToObject(json, id, data = cJSON_CreateObject());
cJSON_AddNumberToObject(data, "x", p->minx);
cJSON_AddNumberToObject(data, "y", p->miny);
@ -85,7 +85,7 @@ int json_export(stream * out, int flags) {
cJSON_AddItemToObject(root, "regions", json = cJSON_CreateObject());
for (r = regions; r; r = r->next) {
cJSON *data;
_snprintf(id, sizeof(id), "%d", r->uid);
sprintf(id, "%d", r->uid); // safe, unless int is bigger than 64 bit
cJSON_AddItemToObject(json, id, data = cJSON_CreateObject());
cJSON_AddNumberToObject(data, "x", r->x);
cJSON_AddNumberToObject(data, "y", r->y);

View File

@ -156,7 +156,10 @@ const char *resourcename(const resource_type * rtype, int flags)
}
if (flags & NMF_PLURAL) {
static char name[64]; // FIXME: static return value
_snprintf(name, sizeof(name), "%s_p", rtype->_name);
size_t len = strlen(rtype->_name);
assert(len <= sizeof(name) - 3);
memcpy(name, rtype->_name, len);
strcpy(name + len, "_p");
return name;
}
return rtype->_name;

View File

@ -523,6 +523,8 @@ static void disable_feature(const char *str) {
char name[32];
int k;
skill_t sk;
size_t len;
sk = findskill(str);
if (sk != NOSKILL) {
enable_skill(sk, false);
@ -534,7 +536,10 @@ static void disable_feature(const char *str) {
enable_keyword(k, false);
return;
}
_snprintf(name, sizeof(name), "%s.enabled", str);
len = strlen(str);
assert(len <= sizeof(name) - 9);
memcpy(name, str, len);
strcpy(name+len, ".enabled");
log_info("disable feature %s\n", name);
config_set(name, "0");
}
@ -796,10 +801,10 @@ static void json_settings(cJSON *json) {
else {
char value[32];
if (child->type == cJSON_Number && child->valuedouble && child->valueint<child->valuedouble) {
_snprintf(value, sizeof(value), "%f", child->valuedouble);
sprintf(value, "%f", child->valuedouble);
}
else {
_snprintf(value, sizeof(value), "%d", child->valueint);
sprintf(value, "%d", child->valueint);
}
config_set(child->string, value);
}

View File

@ -140,37 +140,34 @@ static void test_scale_number(CuTest *tc) {
static void test_unit_name(CuTest *tc) {
unit *u;
char name[32];
test_cleanup();
test_create_world();
u = test_create_unit(test_create_faction(test_create_race("human")), findregion(0, 0));
renumber_unit(u, 666);
unit_setname(u, "Hodor");
_snprintf(name, sizeof(name), "Hodor (%s)", itoa36(u->no));
CuAssertStrEquals(tc, name, unitname(u));
CuAssertStrEquals(tc, "Hodor (ii)", unitname(u));
test_cleanup();
}
static void test_unit_name_from_race(CuTest *tc) {
unit *u;
char name[32];
struct locale *lang;
test_cleanup();
test_create_world();
u = test_create_unit(test_create_faction(test_create_race("human")), findregion(0, 0));
renumber_unit(u, 666);
unit_setname(u, NULL);
lang = get_or_create_locale("de");
locale_setstring(lang, rc_name_s(u->_race, NAME_SINGULAR), "Mensch");
locale_setstring(lang, rc_name_s(u->_race, NAME_PLURAL), "Menschen");
_snprintf(name, sizeof(name), "Mensch (%s)", itoa36(u->no));
CuAssertStrEquals(tc, name, unitname(u));
CuAssertStrEquals(tc, "Mensch (ii)", unitname(u));
CuAssertStrEquals(tc, "Mensch", unit_getname(u));
u->number = 2;
_snprintf(name, sizeof(name), "Menschen (%s)", itoa36(u->no));
CuAssertStrEquals(tc, name, unitname(u));
CuAssertStrEquals(tc, "Menschen (ii)", unitname(u));
CuAssertStrEquals(tc, "Menschen", unit_getname(u));
test_cleanup();

View File

@ -521,7 +521,6 @@ static void test_pay_cmd_other_building(CuTest *tc) {
order *ord;
faction *f;
building *b;
char cmd[32];
test_setup();
setup_pay_cmd(&fix);
@ -531,8 +530,7 @@ static void test_pay_cmd_other_building(CuTest *tc) {
config_set("rules.region_owner_pay_building", "lighthouse");
update_owners(b->region);
_snprintf(cmd, sizeof(cmd), "NOT %s", itoa36(b->no));
ord = create_order(K_PAY, f->locale, cmd);
ord = create_order(K_PAY, f->locale, "NOT %s", itoa36(b->no));
assert(ord);
CuAssertPtrEquals(tc, fix.u1, building_owner(b));
CuAssertIntEquals(tc, 0, pay_cmd(fix.u1, ord));

View File

@ -413,8 +413,10 @@ void nr_spell_syntax(struct stream *out, spellbook_entry * sbe, const struct loc
}
else {
char substr[32];
strncpy(substr, syntaxp, cstr - syntaxp);
substr[cstr - syntaxp] = 0;
size_t len = cstr - syntaxp;
assert(sizeof(substr) > len);
memcpy(substr, syntaxp, len);
substr[len] = 0;
locp = LOC(lang, mkname("spellpar", substr));
syntaxp = substr + 1;
}

View File

@ -849,10 +849,6 @@ const struct unit * u, struct skill * sv, int *dh, int days)
void split_paragraph(strlist ** SP, const char *s, unsigned int indent, unsigned int width, char mark)
{
/* Die Liste SP wird mit dem String s aufgefuellt, mit indent und einer
* mark, falls angegeben. SP wurde also auf 0 gesetzt vor dem Aufruf.
* Vgl. spunit (). */
bool firstline;
static char buf[REPORTWIDTH + 1]; // FIXME: static buffer, artificial limit
size_t len = strlen(s);
@ -883,7 +879,7 @@ void split_paragraph(strlist ** SP, const char *s, unsigned int indent, unsigned
if (!cut) {
cut = s + _min(len, REPORTWIDTH);
}
strncpy(buf + indent, s, cut - s);
memcpy(buf + indent, s, cut - s);
buf[indent + (cut - s)] = 0;
addstrlist(SP, buf); // TODO: too much string copying, cut out this function
while (*cut == ' ') {

View File

@ -65,6 +65,7 @@ int RunAllTests(int argc, char *argv[])
ADD_SUITE(direction);
ADD_SUITE(skill);
ADD_SUITE(keyword);
ADD_SUITE(message);
ADD_SUITE(order);
ADD_SUITE(race);
/* util */

View File

@ -14,7 +14,7 @@ gamedata.test.c
language.test.c
# lists.test.c
# log.test.c
# message.test.c
message.test.c
# nrmessage.test.c
parser.test.c
password.test.c

View File

@ -13,7 +13,7 @@ int wrptr(char **ptr, size_t * size, int result)
{
size_t bytes = (size_t)result;
if (result < 0) {
// _snprintf buffer was too small
// buffer was too small
if (*size > 0) {
**ptr = 0;
*size = 0;

View File

@ -101,16 +101,23 @@ locale *get_or_create_locale(const char *name)
void make_locales(const char *str)
{
const char *tok = str;
while (*tok) {
char zText[32];
while (*tok && *tok != ',')
++tok;
strncpy(zText, str, tok - str);
zText[tok - str] = 0;
get_or_create_locale(zText);
if (*tok) {
str = ++tok;
while (tok) {
char zText[16];
size_t len;
tok = strchr(str, ',');
if (tok) {
len = tok - str;
assert(sizeof(zText) > len);
memcpy(zText, str, len);
str = tok + 1;
}
else {
len = strlen(str);
memcpy(zText, str, len);
}
zText[len] = 0;
get_or_create_locale(zText);
}
}

View File

@ -16,9 +16,20 @@ static void test_language(CuTest *tc)
test_cleanup();
}
static void test_make_locales(CuTest *tc)
{
test_setup();
make_locales("aa,bb,cc");
CuAssertPtrNotNull(tc, get_locale("aa"));
CuAssertPtrNotNull(tc, get_locale("bb"));
CuAssertPtrNotNull(tc, get_locale("cc"));
test_cleanup();
}
CuSuite *get_language_suite(void)
{
CuSuite *suite = CuSuiteNew();
SUITE_ADD_TEST(suite, test_language);
SUITE_ADD_TEST(suite, test_make_locales);
return suite;
}

View File

@ -12,7 +12,7 @@ void log_string(void *data, int level, const char *module, const char *format, v
unused_arg(format);
unused_arg(module);
unused_arg(level);
strncpy(str, arg, 32);
strcpy(str, arg);
}
static void test_logging(CuTest * tc)

View File

@ -87,19 +87,21 @@ message_type *mt_new(const char *name, const char *args[])
for (i = 0; args[i]; ++i) {
const char *x = args[i];
const char *spos = strchr(x, ':');
if (spos == NULL) {
mtype->pnames[i] = _strdup(x);
mtype->types[i] = NULL;
struct arg_type *atype = NULL;
if (spos != NULL) {
atype = find_argtype(spos + 1);
}
if (!atype) {
log_error("unknown argument type %s for message type %s\n", spos + 1, mtype->name);
assert(atype);
}
else {
char *cp = strncpy((char *)malloc(spos - x + 1), x, spos - x);
char *cp;
cp = malloc(spos - x + 1);
memcpy(cp, x, spos - x);
cp[spos - x] = '\0';
mtype->pnames[i] = cp;
mtype->types[i] = find_argtype(spos + 1);
if (mtype->types[i] == NULL) {
log_error("unknown argument type %s for message type %s\n", spos + 1, mtype->name);
}
assert(mtype->types[i]);
mtype->types[i] = atype;
}
}
}

29
src/util/message.test.c Normal file
View File

@ -0,0 +1,29 @@
#include <platform.h>
#include "message.h"
#include <CuTest.h>
#include <tests.h>
static void test_mt_new(CuTest *tc)
{
message_type *mt;
test_setup();
mt = mt_new_va("test", "name:string", "number:int", NULL);
CuAssertPtrNotNull(tc, mt);
CuAssertStrEquals(tc, "test", mt->name);
CuAssertIntEquals(tc, 2, mt->nparameters);
CuAssertPtrNotNull(tc, mt->pnames);
CuAssertStrEquals(tc, "name", mt->pnames[0]);
CuAssertStrEquals(tc, "number", mt->pnames[1]);
CuAssertPtrNotNull(tc, mt->types);
CuAssertStrEquals(tc, "string", mt->types[0]->name);
CuAssertStrEquals(tc, "int", mt->types[1]->name);
test_cleanup();
}
CuSuite *get_message_suite(void)
{
CuSuite *suite = CuSuiteNew();
SUITE_ADD_TEST(suite, test_mt_new);
return suite;
}