[v2,1/3] load_branch_decorations: fix memory leak with non-static filters

Commit Message

Nicolas Guichard Oct. 4, 2024, 11:22 p.m. UTC

From: Nicolas Guichard <[e-mail]>

load_branch_decorations calls normalize_glob_ref on each string of filter's
string_lists. This effectively replaces the potentially non-owning char* of
those items with an owning char*.

Set the strdup_string flag on those string_lists.

This was not caught until now because:
- when passing string_lists already with the strdup_string already set, the
  behaviour was correct
- when passing static string_lists, the new char* remain reachable until
  program exit

Signed-off-by: Nicolas Guichard <[e-mail]>
---
 log-tree.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Eric Sunshine Oct. 5, 2024, 3:43 a.m. UTC | #1

On Fri, Oct 4, 2024 at 7:22 PM Nicolas Guichard via GitGitGadget
<[e-mail]> wrote:
> load_branch_decorations calls normalize_glob_ref on each string of filter's
> string_lists. This effectively replaces the potentially non-owning char* of
> those items with an owning char*.
>
> Set the strdup_string flag on those string_lists.
>
> This was not caught until now because:
> - when passing string_lists already with the strdup_string already set, the
>   behaviour was correct
> - when passing static string_lists, the new char* remain reachable until
>   program exit
>
> Signed-off-by: Nicolas Guichard <[e-mail]>

Not a proper review; just a couple style nits below...

> diff --git a/log-tree.c b/log-tree.c
> @@ -232,6 +232,11 @@ void load_ref_decorations(struct decoration_filter *filter, int flags)
> +                       // normalize_glob_ref duplicates the strings
> +                       filter->exclude_ref_pattern->strdup_strings = true;
> +                       filter->include_ref_pattern->strdup_strings = true;
> +                       filter->exclude_ref_config_pattern->strdup_strings = true;

For the sake of older platforms and older compilers, we're still
avoiding certain modernisms on this project. Instead:

* use /* ... */ style comments instead of // style
* use 1 instead of `true` (and 0 instead of `false`)

Patch

diff --git a/log-tree.c b/log-tree.c
index 3758e0d3b8e..cd57de2424e 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -232,6 +232,11 @@  void load_ref_decorations(struct decoration_filter *filter, int flags)
 			for_each_string_list_item(item, filter->exclude_ref_config_pattern) {
 				normalize_glob_ref(item, NULL, item->string);
 			}
+
+			// normalize_glob_ref duplicates the strings
+			filter->exclude_ref_pattern->strdup_strings = true;
+			filter->include_ref_pattern->strdup_strings = true;
+			filter->exclude_ref_config_pattern->strdup_strings = true;
 		}
 		decoration_loaded = 1;
 		decoration_flags = flags;