gronod/OpenTTD
1
0
Fork 0
mirror of https://github.com/OpenTTD/OpenTTD.git synced 2025-02-11 16:54:42 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix: memory leak when parsing (strgen) commands by moving to C++ containers

Browse Source
This commit is contained in:
Rubidium 2023-04-30 19:17:42 +02:00 committed by rubidium42
parent 4f94655cc2
commit 802d6cb509
3 changed files with 35 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
src/game/game_text.cpp
View File

@ -270,10 +270,9 @@ static void ExtractStringParams(const StringData &data, StringParamsList &params
if (ls != nullptr) { if (ls != nullptr) {
StringParams &param = params.emplace_back(); StringParams &param = params.emplace_back();
ParsedCommandStruct pcs; ParsedCommandStruct pcs = ExtractCommandString(ls->english.c_str(), false);
ExtractCommandString(&pcs, ls->english.c_str(), false);
for (const CmdStruct *cs : pcs.cmd) { for (const CmdStruct *cs : pcs.consuming_commands) {
if (cs == nullptr) break; if (cs == nullptr) break;
param.emplace_back(GetParamType(cs), cs->consumes); param.emplace_back(GetParamType(cs), cs->consumes);
} }

11
src/strgen/strgen.h
View File

@ -137,18 +137,17 @@ struct LanguageWriter {
struct CmdStruct; struct CmdStruct;
struct CmdPair { struct CmdPair {
const CmdStruct *a; const CmdStruct *cmd;
const char *v; std::string param;
}; };
struct ParsedCommandStruct { struct ParsedCommandStruct {
uint np; std::vector<CmdPair> non_consuming_commands;
CmdPair pairs[32]; std::array<const CmdStruct*, 32> consuming_commands{ nullptr }; // ordered by param #
const CmdStruct *cmd[32]; // ordered by param #
}; };
const CmdStruct *TranslateCmdForCompare(const CmdStruct *a); const CmdStruct *TranslateCmdForCompare(const CmdStruct *a);
void ExtractCommandString(ParsedCommandStruct *p, const char *s, bool warnings); ParsedCommandStruct ExtractCommandString(const char *s, bool warnings);
void StrgenWarningI(const std::string &msg); void StrgenWarningI(const std::string &msg);
void StrgenErrorI(const std::string &msg); void StrgenErrorI(const std::string &msg);

60
src/strgen/strgen_base.cpp
View File

@ -336,7 +336,7 @@ void EmitPlural(Buffer *buffer, char *buf, int value)
/* Parse out the number, if one exists. Otherwise default to prev arg. */ /* Parse out the number, if one exists. Otherwise default to prev arg. */
if (!ParseRelNum(&buf, &argidx, &offset)) argidx--; if (!ParseRelNum(&buf, &argidx, &offset)) argidx--;
const CmdStruct *cmd = _cur_pcs.cmd[argidx]; const CmdStruct *cmd = _cur_pcs.consuming_commands[argidx];
if (offset == -1) { if (offset == -1) {
/* Use default offset */ /* Use default offset */
if (cmd == nullptr || cmd->default_plural_offset < 0) { if (cmd == nullptr || cmd->default_plural_offset < 0) {
@ -401,7 +401,7 @@ void EmitGender(Buffer *buffer, char *buf, int value)
* If no relative number exists, default to +0 */ * If no relative number exists, default to +0 */
ParseRelNum(&buf, &argidx, &offset); ParseRelNum(&buf, &argidx, &offset);
const CmdStruct *cmd = _cur_pcs.cmd[argidx]; const CmdStruct *cmd = _cur_pcs.consuming_commands[argidx];
if (cmd == nullptr || (cmd->flags & C_GENDER) == 0) { if (cmd == nullptr || (cmd->flags & C_GENDER) == 0) {
StrgenFatal("Command '{}' can't have a gender", cmd == nullptr ? "<empty>" : cmd->cmd); StrgenFatal("Command '{}' can't have a gender", cmd == nullptr ? "<empty>" : cmd->cmd);
} }
@ -531,14 +531,14 @@ StringReader::StringReader(StringData &data, const std::string &file, bool maste
{ {
} }
void ExtractCommandString(ParsedCommandStruct *p, const char *s, bool warnings) ParsedCommandStruct ExtractCommandString(const char *s, bool warnings)
{ {
char param[MAX_COMMAND_PARAM_SIZE]; char param[MAX_COMMAND_PARAM_SIZE];
int argno; int argno;
int argidx = 0; int argidx = 0;
int casei; int casei;
memset(p, 0, sizeof(*p)); ParsedCommandStruct p;
for (;;) { for (;;) {
/* read until next command from a. */ /* read until next command from a. */
@ -551,17 +551,16 @@ void ExtractCommandString(ParsedCommandStruct *p, const char *s, bool warnings)
if (ar->consumes) { if (ar->consumes) {
if (argno != -1) argidx = argno; if (argno != -1) argidx = argno;
if (argidx < 0 || (uint)argidx >= lengthof(p->cmd)) StrgenFatal("invalid param idx {}", argidx); if (argidx < 0 || (uint)argidx >= p.consuming_commands.max_size()) StrgenFatal("invalid param idx {}", argidx);
if (p->cmd[argidx] != nullptr && p->cmd[argidx] != ar) StrgenFatal("duplicate param idx {}", argidx); if (p.consuming_commands[argidx] != nullptr && p.consuming_commands[argidx] != ar) StrgenFatal("duplicate param idx {}", argidx);
p->cmd[argidx++] = ar; p.consuming_commands[argidx++] = ar;
} else if (!(ar->flags & C_DONTCOUNT)) { // Ignore some of them } else if (!(ar->flags & C_DONTCOUNT)) { // Ignore some of them
if (p->np >= lengthof(p->pairs)) StrgenFatal("too many commands in string, max {}", lengthof(p->pairs)); p.non_consuming_commands.emplace_back(CmdPair{ar, param});
p->pairs[p->np].a = ar;
p->pairs[p->np].v = param[0] != '\0' ? stredup(param) : "";
p->np++;
} }
} }
return p;
} }
@ -592,45 +591,42 @@ static bool CheckCommandsMatch(const char *a, const char *b, const char *name)
*/ */
if (!_translation) return true; if (!_translation) return true;
ParsedCommandStruct templ;
ParsedCommandStruct lang;
bool result = true; bool result = true;
ExtractCommandString(&templ, b, true); ParsedCommandStruct templ = ExtractCommandString(b, true);
ExtractCommandString(&lang, a, true); ParsedCommandStruct lang = ExtractCommandString(a, true);
/* For each string in templ, see if we find it in lang */ /* For each string in templ, see if we find it in lang */
if (templ.np != lang.np) { if (templ.non_consuming_commands.max_size() != lang.non_consuming_commands.max_size()) {
StrgenWarning("{}: template string and language string have a different # of commands", name); StrgenWarning("{}: template string and language string have a different # of commands", name);
result = false; result = false;
} }
for (uint i = 0; i < templ.np; i++) { for (auto &templ_nc : templ.non_consuming_commands) {
/* see if we find it in lang, and zero it out */ /* see if we find it in lang, and zero it out */
bool found = false; bool found = false;
for (uint j = 0; j < lang.np; j++) { for (auto &lang_nc : lang.non_consuming_commands) {
if (templ.pairs[i].a == lang.pairs[j].a && if (templ_nc.cmd == lang_nc.cmd && templ_nc.param == lang_nc.param) {
strcmp(templ.pairs[i].v, lang.pairs[j].v) == 0) {
/* it was found in both. zero it out from lang so we don't find it again */ /* it was found in both. zero it out from lang so we don't find it again */
lang.pairs[j].a = nullptr; lang_nc.cmd = nullptr;
found = true; found = true;
break; break;
} }
} }
if (!found) { if (!found) {
StrgenWarning("{}: command '{}' exists in template file but not in language file", name, templ.pairs[i].a->cmd); StrgenWarning("{}: command '{}' exists in template file but not in language file", name, templ_nc.cmd->cmd);
result = false; result = false;
} }
} }
/* if we reach here, all non consumer commands match up. /* if we reach here, all non consumer commands match up.
* Check if the non consumer commands match up also. */ * Check if the non consumer commands match up also. */
for (uint i = 0; i < lengthof(templ.cmd); i++) { for (uint i = 0; i < templ.consuming_commands.max_size(); i++) {
if (TranslateCmdForCompare(templ.cmd[i]) != lang.cmd[i]) { if (TranslateCmdForCompare(templ.consuming_commands[i]) != lang.consuming_commands[i]) {
StrgenWarning("{}: Param idx #{} '{}' doesn't match with template command '{}'", name, i, StrgenWarning("{}: Param idx #{} '{}' doesn't match with template command '{}'", name, i,
lang.cmd[i] == nullptr ? "<empty>" : TranslateCmdForCompare(lang.cmd[i])->cmd, lang.consuming_commands[i] == nullptr ? "<empty>" : TranslateCmdForCompare(lang.consuming_commands[i])->cmd,
templ.cmd[i] == nullptr ? "<empty>" : templ.cmd[i]->cmd); templ.consuming_commands[i] == nullptr ? "<empty>" : templ.consuming_commands[i]->cmd);
result = false; result = false;
} }
} }
@ -801,20 +797,20 @@ static int TranslateArgumentIdx(int argidx, int offset)
{ {
int sum; int sum;
if (argidx < 0 || (uint)argidx >= lengthof(_cur_pcs.cmd)) { if (argidx < 0 || (uint)argidx >= _cur_pcs.consuming_commands.max_size()) {
StrgenFatal("invalid argidx {}", argidx); StrgenFatal("invalid argidx {}", argidx);
} }
const CmdStruct *cs = _cur_pcs.cmd[argidx]; const CmdStruct *cs = _cur_pcs.consuming_commands[argidx];
if (cs != nullptr && cs->consumes <= offset) { if (cs != nullptr && cs->consumes <= offset) {
StrgenFatal("invalid argidx offset {}:{}", argidx, offset); StrgenFatal("invalid argidx offset {}:{}", argidx, offset);
} }
if (_cur_pcs.cmd[argidx] == nullptr) { if (_cur_pcs.consuming_commands[argidx] == nullptr) {
StrgenFatal("no command for this argidx {}", argidx); StrgenFatal("no command for this argidx {}", argidx);
} }
for (int i = sum = 0; i < argidx; i++) { for (int i = sum = 0; i < argidx; i++) {
cs = _cur_pcs.cmd[i]; cs = _cur_pcs.consuming_commands[i];
sum += (cs != nullptr) ? cs->consumes : 1; sum += (cs != nullptr) ? cs->consumes : 1;
} }
@ -860,7 +856,7 @@ static void PutCommandString(Buffer *buffer, const char *str)
} }
/* Output the one from the master string... it's always accurate. */ /* Output the one from the master string... it's always accurate. */
cs = _cur_pcs.cmd[_cur_argidx++]; cs = _cur_pcs.consuming_commands[_cur_argidx++];
if (cs == nullptr) { if (cs == nullptr) {
StrgenFatal("{}: No argument exists at position {}", _cur_ident, _cur_argidx - 1); StrgenFatal("{}: No argument exists at position {}", _cur_ident, _cur_argidx - 1);
} }
@ -942,7 +938,7 @@ void LanguageWriter::WriteLang(const StringData &data)
} }
/* Extract the strings and stuff from the english command string */ /* Extract the strings and stuff from the english command string */
ExtractCommandString(&_cur_pcs, ls->english.c_str(), false); _cur_pcs = ExtractCommandString(ls->english.c_str(), false);
if (!ls->translated_cases.empty() || !ls->translated.empty()) { if (!ls->translated_cases.empty() || !ls->translated.empty()) {
cmdp = &ls->translated; cmdp = &ls->translated;