From 9620d1693a593ff30e933a440af2036e450e9da9 Mon Sep 17 00:00:00 2001 From: Marko Lindqvist Date: Sun, 11 Sep 2022 10:11:23 +0300 Subject: [PATCH 27/27] Fix memory leaks from req_to_fstring() usage See osdn #45544 Signed-off-by: Marko Lindqvist --- common/actions.c | 15 +++++++++++---- common/requirements.c | 23 +++++++++++++++-------- common/requirements.h | 8 ++++++-- server/rssanity.c | 10 ++++++++-- tools/ruleutil/rulesave.c | 5 ++++- 5 files changed, 44 insertions(+), 17 deletions(-) diff --git a/common/actions.c b/common/actions.c index 3436352b2f..d359c9fca3 100644 --- a/common/actions.c +++ b/common/actions.c @@ -1440,8 +1440,9 @@ enabler_first_self_contradiction(const struct action_enabler *enabler) struct requirement *local_diplrel; struct requirement *unclaimed_req; struct requirement tile_is_claimed; - struct action *paction = action_by_number(enabler->action); + struct astring astr1; + struct astring astr2; if (action_get_target_kind(paction) != ATK_TILE) { /* Not tile targeted */ @@ -1478,7 +1479,11 @@ enabler_first_self_contradiction(const struct action_enabler *enabler) N_("In enabler for \"%s\": No diplomatic relation to Nature." " Requirements {%s} and {%s} contradict each other."), action_rule_name(paction), - req_to_fstring(local_diplrel), req_to_fstring(unclaimed_req)); + req_to_fstring(local_diplrel, &astr1), + req_to_fstring(unclaimed_req, &astr2)); + + astr_free(&astr1); + astr_free(&astr2); /* The first suggestion is to remove the diplrel */ out->suggested_solutions[0].req = *local_diplrel; @@ -2785,8 +2790,8 @@ enabler_first_clarification(const struct action_enabler *enabler) struct requirement *claimed_req; struct requirement tile_is_claimed; struct requirement tile_is_unclaimed; - struct action *paction = action_by_number(enabler->action); + struct astring astr; if (action_get_target_kind(paction) != ATK_TILE) { /* Not tile targeted */ @@ -2830,7 +2835,9 @@ enabler_first_clarification(const struct action_enabler *enabler) * so this is implicit.) */ N_("Possible clarification: Requirement {%s} of action \"%s\" " "implies a claimed tile. No diplomatic relation to Nature."), - req_to_fstring(local_diplrel), action_rule_name(paction)); + req_to_fstring(local_diplrel, &astr), action_rule_name(paction)); + + astr_free(&astr); /* The solution is to add the requirement that the tile is claimed */ out->suggested_solutions[0].req = tile_is_claimed; diff --git a/common/requirements.c b/common/requirements.c index a45024f9ef..ab7b88a9c4 100644 --- a/common/requirements.c +++ b/common/requirements.c @@ -657,18 +657,17 @@ int universal_number(const struct universal *source) Returns the given requirement as a formatted string ready for printing. Does not care about the 'quiet' property. ****************************************************************************/ -const char *req_to_fstring(const struct requirement *req) +const char *req_to_fstring(const struct requirement *req, + struct astring *astr) { - struct astring printable_req = ASTRING_INIT; - - astr_set(&printable_req, "%s%s %s %s%s", + astr_set(astr, "%s%s %s %s%s", req->survives ? "surviving " : "", req_range_name(req->range), universal_type_rule_name(&req->source), req->present ? "" : "!", universal_rule_name(&req->source)); - return astr_str(&printable_req); + return astr_str(astr); } /**************************************************************************** @@ -3803,6 +3802,7 @@ const char *req_vec_change_translation(const struct req_vec_change *change, { const char *req_vec_description; static char buf[MAX_LEN_NAME * 3]; + struct astring astr; fc_assert_ret_val(change, NULL); fc_assert_ret_val(req_vec_change_operation_is_valid(change->operation), @@ -3830,8 +3830,9 @@ const char *req_vec_change_translation(const struct req_vec_change *change, * like "actor_reqs" */ _("%s %s from %s"), req_vec_change_operation_name(change->operation), - req_to_fstring(&change->req), + req_to_fstring(&change->req, &astr), req_vec_description); + astr_free(&astr); break; case RVCO_APPEND: fc_snprintf(buf, sizeof(buf), @@ -3843,8 +3844,9 @@ const char *req_vec_change_translation(const struct req_vec_change *change, * like "actor_reqs" */ _("%s %s to %s"), req_vec_change_operation_name(change->operation), - req_to_fstring(&change->req), + req_to_fstring(&change->req, &astr), req_vec_description); + astr_free(&astr); break; case RVCO_NOOP: fc_snprintf(buf, sizeof(buf), @@ -4011,10 +4013,15 @@ req_vec_get_first_contradiction(const struct requirement_vector *vec, if (are_requirements_contradictions(preq, nreq)) { struct req_vec_problem *problem; + struct astring astr; + struct astring nastr; problem = req_vec_problem_new(2, N_("Requirements {%s} and {%s} contradict each other."), - req_to_fstring(preq), req_to_fstring(nreq)); + req_to_fstring(preq, &astr), req_to_fstring(nreq, &nastr)); + + astr_free(&astr); + astr_free(&nastr); /* The solution is to remove one of the contradictions. */ problem->suggested_solutions[0].operation = RVCO_REMOVE; diff --git a/common/requirements.h b/common/requirements.h index bd284d39d3..ec5b45c41b 100644 --- a/common/requirements.h +++ b/common/requirements.h @@ -18,6 +18,9 @@ extern "C" { #endif /* __cplusplus */ +/* utility */ +#include "astring.h" + /* common */ #include "fc_types.h" @@ -90,7 +93,8 @@ struct requirement { struct requirement req_from_str(const char *type, const char *range, bool survives, bool present, bool quiet, const char *value); -const char *req_to_fstring(const struct requirement *req); +const char *req_to_fstring(const struct requirement *req, + struct astring *astr); void req_get_values(const struct requirement *req, int *type, int *range, bool *survives, bool *present, bool *quiet, @@ -100,7 +104,7 @@ struct requirement req_from_values(int type, int range, int value); bool are_requirements_equal(const struct requirement *req1, - const struct requirement *req2); + const struct requirement *req2); bool are_requirements_contradictions(const struct requirement *req1, const struct requirement *req2); diff --git a/server/rssanity.c b/server/rssanity.c index 415488382b..ef2d794f07 100644 --- a/server/rssanity.c +++ b/server/rssanity.c @@ -821,6 +821,8 @@ bool sanity_check_ruleset_data(struct rscompat_info *compat) advance_rule_name(padvance)); ok = FALSE; } else if (!is_req_unchanging(preq)) { + struct astring astr; + /* Only support unchanging requirements until the reachability code * can handle it and the tech tree can display changing * requirements. */ @@ -830,7 +832,8 @@ bool sanity_check_ruleset_data(struct rscompat_info *compat) " the game. Changing requirements aren't supported" " yet.", advance_rule_name(padvance), - req_to_fstring(preq)); + req_to_fstring(preq, &astr)); + astr_free(&astr); ok = FALSE; } } requirement_vector_iterate_end; @@ -1210,6 +1213,8 @@ bool sanity_check_ruleset_data(struct rscompat_info *compat) requirement_vector_iterate(&(enabler->target_reqs), preq) { if (preq->source.kind == VUT_DIPLREL && preq->range == REQ_RANGE_LOCAL) { + struct astring astr; + /* A Local DiplRel requirement can be expressed as a requirement * in actor_reqs. Demand that it is there. This avoids breaking * code that reasons about actions. */ @@ -1219,7 +1224,8 @@ bool sanity_check_ruleset_data(struct rscompat_info *compat) "section \"Requirement vector rules\" in " "doc/README.actions", action_id_rule_name(act), - req_to_fstring(preq)); + req_to_fstring(preq, &astr)); + astr_free(&astr); ok = FALSE; } } requirement_vector_iterate_end; diff --git a/tools/ruleutil/rulesave.c b/tools/ruleutil/rulesave.c index 3f8de307ea..ff8be943d4 100644 --- a/tools/ruleutil/rulesave.c +++ b/tools/ruleutil/rulesave.c @@ -532,8 +532,11 @@ static bool save_action_auto_uflag_block(struct section_file *sfile, protecor_flag[i++] = req->source.value.unitflag; } else if (unexpected_req(req)) { + struct astring astr; + log_error("Can't handle action auto performer requirement %s", - req_to_fstring(req)); + req_to_fstring(req, &astr)); + astr_free(&astr); return FALSE; } -- 2.35.1