Skip to content

Commit f3a9680

Browse files
ahuntgitster
authored andcommitted
mailinfo: also free strbuf lists when clearing mailinfo
mailinfo.p_hdr_info/s_hdr_info are null-terminated lists of strbuf's, with entries pointing either to NULL or an allocated strbuf. Therefore we need to free those strbuf's (and not just the data they contain) whenever we're done with a given entry. (See handle_header() where those new strbufs are malloc'd.) Once we no longer need the list (and not just its entries) we can switch over to strbuf_list_free() instead of manually iterating over the list, which takes care of those additional details for us. We can only do this in clear_mailinfo() - in handle_commit_message() we are only clearing the array contents but want to reuse the array itself, hence we can't use strbuf_list_free() there. However, strbuf_list_free() cannot handle a NULL input, and the lists we are freeing might be NULL. Therefore we add a NULL check in strbuf_list_free() to make it safe to use with a NULL input (which is a pattern used by some of the other *_free() functions around git). Leak output from t0023: Direct leak of 72 byte(s) in 3 object(s) allocated from: #0 0x49a85d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3 #1 0x9ac9f4 in do_xmalloc wrapper.c:41:8 #2 0x9ac9ca in xmalloc wrapper.c:62:9 #3 0x7f6cf7 in handle_header mailinfo.c:205:10 #4 0x7f5abf in check_header mailinfo.c:583:4 #5 0x7f5524 in mailinfo mailinfo.c:1197:3 #6 0x4dcc95 in parse_mail builtin/am.c:1167:6 #7 0x4d9070 in am_run builtin/am.c:1732:12 #8 0x4d5b7a in cmd_am builtin/am.c:2398:3 #9 0x4cd91d in run_builtin git.c:467:11 #10 0x4cb5f3 in handle_builtin git.c:719:3 #11 0x4ccf47 in run_argv git.c:808:4 #12 0x4caf49 in cmd_main git.c:939:19 #13 0x69e43e in main common-main.c:52:11 #14 0x7fc1fadfa349 in __libc_start_main (/lib64/libc.so.6+0x24349) SUMMARY: AddressSanitizer: 72 byte(s) leaked in 3 allocation(s). Signed-off-by: Andrzej Hunt <ajrhunt@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent 52a9436 commit f3a9680

File tree

2 files changed

+5
-11
lines changed

2 files changed

+5
-11
lines changed

mailinfo.c

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -821,7 +821,7 @@ static int handle_commit_msg(struct mailinfo *mi, struct strbuf *line)
821821
for (i = 0; header[i]; i++) {
822822
if (mi->s_hdr_data[i])
823823
strbuf_release(mi->s_hdr_data[i]);
824-
mi->s_hdr_data[i] = NULL;
824+
FREE_AND_NULL(mi->s_hdr_data[i]);
825825
}
826826
return 0;
827827
}
@@ -1236,22 +1236,14 @@ void setup_mailinfo(struct mailinfo *mi)
12361236

12371237
void clear_mailinfo(struct mailinfo *mi)
12381238
{
1239-
int i;
1240-
12411239
strbuf_release(&mi->name);
12421240
strbuf_release(&mi->email);
12431241
strbuf_release(&mi->charset);
12441242
strbuf_release(&mi->inbody_header_accum);
12451243
free(mi->message_id);
12461244

1247-
if (mi->p_hdr_data)
1248-
for (i = 0; mi->p_hdr_data[i]; i++)
1249-
strbuf_release(mi->p_hdr_data[i]);
1250-
free(mi->p_hdr_data);
1251-
if (mi->s_hdr_data)
1252-
for (i = 0; mi->s_hdr_data[i]; i++)
1253-
strbuf_release(mi->s_hdr_data[i]);
1254-
free(mi->s_hdr_data);
1245+
strbuf_list_free(mi->p_hdr_data);
1246+
strbuf_list_free(mi->s_hdr_data);
12551247

12561248
while (mi->content < mi->content_top) {
12571249
free(*(mi->content_top));

strbuf.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,8 @@ void strbuf_list_free(struct strbuf **sbs)
209209
{
210210
struct strbuf **s = sbs;
211211

212+
if (!s)
213+
return;
212214
while (*s) {
213215
strbuf_release(*s);
214216
free(*s++);

0 commit comments

Comments
 (0)