diff mbox series

[v2,01/12] fmt-merge-msg: free newly allocated temporary strings when done

Message ID 20210725130830.5145-2-andrzej@ahunt.org (mailing list archive)
State Accepted
Commit 9fa62137314437162bf3e022f44c1fa8e5b43b50
Headers show
Series [v2,01/12] fmt-merge-msg: free newly allocated temporary strings when done | expand

Commit Message

Andrzej Hunt July 25, 2021, 1:08 p.m. UTC 
From: Andrzej Hunt <ajrhunt@google.com>

origin starts off pointing to somewhere within line, which is owned by
the caller. Later we might allocate a new string using xmemdupz() or
xstrfmt(). To avoid leaking these new strings, we introduce a to_free
pointer - which allows us to safely free the newly allocated string when
we're done (we cannot just free origin directly as it might still be
pointing to line).

LSAN output from t0090:

Direct leak of 8 byte(s) in 1 object(s) allocated from:
    #0 0x49a82d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
    #1 0xa71f49 in do_xmalloc wrapper.c:41:8
    #2 0xa720b0 in do_xmallocz wrapper.c:75:8
    #3 0xa720b0 in xmallocz wrapper.c:83:9
    #4 0xa720b0 in xmemdupz wrapper.c:99:16
    #5 0x8092ba in handle_line fmt-merge-msg.c:187:23
    #6 0x8092ba in fmt_merge_msg fmt-merge-msg.c:666:7
    #7 0x5ce2e6 in prepare_merge_message builtin/merge.c:1119:2
    #8 0x5ce2e6 in collect_parents builtin/merge.c:1215:3
    #9 0x5c9c1e in cmd_merge builtin/merge.c:1454:16
    #10 0x4ce83e in run_builtin git.c:475:11
    #11 0x4ccafe in handle_builtin git.c:729:3
    #12 0x4cb01c in run_argv git.c:818:4
    #13 0x4cb01c in cmd_main git.c:949:19
    #14 0x6b3fad in main common-main.c:52:11
    #15 0x7fb929620349 in __libc_start_main (/lib64/libc.so.6+0x24349)

SUMMARY: AddressSanitizer: 8 byte(s) leaked in 1 allocation(s).

Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
---
 fmt-merge-msg.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Junio C Hamano July 26, 2021, 7:20 p.m. UTC | #1 
andrzej@ahunt.org writes:

> diff --git a/fmt-merge-msg.c b/fmt-merge-msg.c
> index 0f66818e0f..b969dc6ebb 100644
> --- a/fmt-merge-msg.c
> +++ b/fmt-merge-msg.c
> @@ -108,6 +108,7 @@ static int handle_line(char *line, struct merge_parents *merge_parents)
>  	struct origin_data *origin_data;
>  	char *src;
>  	const char *origin, *tag_name;
> +	char *to_free = NULL;
>  	struct src_data *src_data;
>  	struct string_list_item *item;
>  	int pulling_head = 0;
> @@ -183,12 +184,13 @@ static int handle_line(char *line, struct merge_parents *merge_parents)
>  	if (!strcmp(".", src) || !strcmp(src, origin)) {
>  		int len = strlen(origin);
>  		if (origin[0] == '\'' && origin[len - 1] == '\'')
> -			origin = xmemdupz(origin + 1, len - 2);
> +			origin = to_free = xmemdupz(origin + 1, len - 2);
>  	} else
> -		origin = xstrfmt("%s of %s", origin, src);
> +		origin = to_free = xstrfmt("%s of %s", origin, src);
>  	if (strcmp(".", src))
>  		origin_data->is_local_branch = 0;
>  	string_list_append(&origins, origin)->util = origin_data;
> +	free(to_free);
>  	return 0;
>  }

Thanks; obviously correct.
diff mbox series

Patch

diff --git a/fmt-merge-msg.c b/fmt-merge-msg.c
index 0f66818e0f..b969dc6ebb 100644
--- a/fmt-merge-msg.c
+++ b/fmt-merge-msg.c
@@ -108,6 +108,7 @@  static int handle_line(char *line, struct merge_parents *merge_parents)
 	struct origin_data *origin_data;
 	char *src;
 	const char *origin, *tag_name;
+	char *to_free = NULL;
 	struct src_data *src_data;
 	struct string_list_item *item;
 	int pulling_head = 0;
@@ -183,12 +184,13 @@  static int handle_line(char *line, struct merge_parents *merge_parents)
 	if (!strcmp(".", src) || !strcmp(src, origin)) {
 		int len = strlen(origin);
 		if (origin[0] == '\'' && origin[len - 1] == '\'')
-			origin = xmemdupz(origin + 1, len - 2);
+			origin = to_free = xmemdupz(origin + 1, len - 2);
 	} else
-		origin = xstrfmt("%s of %s", origin, src);
+		origin = to_free = xstrfmt("%s of %s", origin, src);
 	if (strcmp(".", src))
 		origin_data->is_local_branch = 0;
 	string_list_append(&origins, origin)->util = origin_data;
+	free(to_free);
 	return 0;
 }