From 3e8746e78b07d6873ad9ce0e66f0ac4a5597a75d Mon Sep 17 00:00:00 2001 From: Krystine Sherwin <93062060+KrystalDelusion@users.noreply.github.com> Date: Wed, 23 Jul 2025 12:24:14 +1200 Subject: [PATCH] log_help: Fix mem leaks `_content` vector owns elements so that when the `ContentListing` is deleted so is the content. Remove `get_content()` method in favour of `begin()` and `end()` const iterators. More `const` in general, and iterations over `ContentListing` use `&content` to avoid copying data. --- kernel/log_help.cc | 27 +++++++++++++-------------- kernel/log_help.h | 36 ++++++++++++++++-------------------- kernel/register.cc | 4 ++-- 3 files changed, 31 insertions(+), 36 deletions(-) diff --git a/kernel/log_help.cc b/kernel/log_help.cc index 45228b024..30c06a7c3 100644 --- a/kernel/log_help.cc +++ b/kernel/log_help.cc @@ -21,7 +21,7 @@ USING_YOSYS_NAMESPACE -Json ContentListing::to_json() { +Json ContentListing::to_json() const { Json::object object; object["type"] = type; if (body.length()) object["body"] = body; @@ -29,7 +29,7 @@ Json ContentListing::to_json() { if (source_line != 0) object["source_line"] = source_line; object["options"] = Json(options); Json::array content_array; - for (auto child : _content) content_array.push_back(child->to_json()); + for (auto child : _content) content_array.push_back(child.to_json()); object["content"] = content_array; return object; } @@ -73,9 +73,8 @@ ContentListing* ContentListing::open_option(const string &text, const source_location location) { log_assert(type.compare("root") == 0 || type.compare("usage") == 0); - auto option = new ContentListing("option", text, location); - add_content(option); - return option; + add_content("option", text, location); + return back(); } #define MAX_LINE_LEN 80 @@ -131,20 +130,20 @@ PrettyHelp *PrettyHelp::get_current() return current_help; } -void PrettyHelp::log_help() +void PrettyHelp::log_help() const { - for (auto content : _root_listing.get_content()) { - if (content->type.compare("usage") == 0) { - log_pass_str(content->body, 1, true); + for (auto &content : _root_listing) { + if (content.type.compare("usage") == 0) { + log_pass_str(content.body, 1, true); log("\n"); - } else if (content->type.compare("option") == 0) { - log_pass_str(content->body, 1); - for (auto text : content->get_content()) { - log_pass_str(text->body, 2); + } else if (content.type.compare("option") == 0) { + log_pass_str(content.body, 1); + for (auto text : content) { + log_pass_str(text.body, 2); log("\n"); } } else { - log_pass_str(content->body, 0); + log_pass_str(content.body, 0); log("\n"); } } diff --git a/kernel/log_help.h b/kernel/log_help.h index 0a40fc531..3ce0ac7aa 100644 --- a/kernel/log_help.h +++ b/kernel/log_help.h @@ -26,7 +26,7 @@ YOSYS_NAMESPACE_BEGIN class ContentListing { - vector _content; + vector _content; public: string type; string body; @@ -45,22 +45,17 @@ public: ContentListing(string type, string body, source_location location) : ContentListing(type, body, location.file_name(), location.line()) { } - void add_content(ContentListing *new_content) { - _content.push_back(new_content); - } - void add_content(string type, string body, source_location location) { - auto new_content = new ContentListing(type, body, location); - add_content(new_content); + _content.push_back({type, body, location}); } - bool has_content() { return _content.size() != 0; } - const vector get_content() { - const vector content = _content; - return content; - } + bool has_content() const { return _content.size() != 0; } + + vector::const_iterator begin() const { return _content.cbegin(); } + vector::const_iterator end() const { return _content.cend(); } + ContentListing* back() { - return _content.back(); + return &_content.back(); } void set_option(string key, string val = "") { @@ -95,7 +90,7 @@ public: const source_location location = source_location::current() ); - Json to_json(); + Json to_json() const; }; class PrettyHelp @@ -113,18 +108,19 @@ public: static PrettyHelp *get_current(); - bool has_content() { return _root_listing.has_content(); } - const vector get_content() { - return _root_listing.get_content(); - } + bool has_content() const { return _root_listing.has_content(); } + + vector::const_iterator begin() const { return _root_listing.begin(); } + vector::const_iterator end() const { return _root_listing.end(); } + ContentListing* get_root() { return &_root_listing; } void set_group(const string g) { group = g; } - bool has_group() { return group.compare("unknown") != 0; } + bool has_group() const { return group.compare("unknown") != 0; } - void log_help(); + void log_help() const; }; YOSYS_NAMESPACE_END diff --git a/kernel/register.cc b/kernel/register.cc index ea2a2624f..0b02a6aa5 100644 --- a/kernel/register.cc +++ b/kernel/register.cc @@ -923,8 +923,8 @@ struct HelpPass : public Pass { json.name(name.c_str()); json.begin_object(); json.entry("title", title); json.name("content"); json.begin_array(); - for (auto content : cmd_help.get_content()) - json.value(content->to_json()); + for (auto &content : cmd_help) + json.value(content.to_json()); json.end_array(); json.entry("group", cmd_help.group); json.entry("source_file", pass->location.file_name());