range-diff: plug memory leak in read_patches()

Amend code added in d9c66f0b5bf (range-diff: first rudimentary implementation, 2018-08-13) to use a "goto cleanup" pattern. This makes for less code, and frees memory that we'd previously leak. The reason for changing free(util) to FREE_AND_NULL(util) is because at the end of the function we append the contents of "util" to a "struct string_list" if it's non-NULL. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ævar Arnfjörð Bjarmason committed Mar 4, 2022 at 19:32 UTC 2d102c2bca1a0c50f17108189f134279cad941cd
1 file changed +12 -17
range-diff.c
+12 -17
@@ -40,6 +40,7 @@ static int read_patches(const char *range, struct string_list *list,
40 char *line, *current_filename = NULL;
41 ssize_t len;
42 size_t size;
43 + int ret = -1;
44
45 strvec_pushl(&cp.args, "log", "--no-color", "-p", "--no-merges",
46 "--reverse", "--date-order", "--decorate=no",
@@ -68,10 +69,10 @@ static int read_patches(const char *range, struct string_list *list,
69 if (strbuf_read(&contents, cp.out, 0) < 0) {
70 error_errno(_("could not read `log` output"));
71 finish_command(&cp);
71 - return -1;
72 + goto cleanup;
73 }
74 if (finish_command(&cp))
74 - return -1;
75 + goto cleanup;
76
77 line = contents.buf;
78 size = contents.len;
@@ -95,12 +96,9 @@ static int read_patches(const char *range, struct string_list *list,
96 CALLOC_ARRAY(util, 1);
97 if (get_oid(p, &util->oid)) {
98 error(_("could not parse commit '%s'"), p);
98 - free(util);
99 - free(current_filename);
99 + FREE_AND_NULL(util);
100 string_list_clear(list, 1);
101 - strbuf_release(&buf);
102 - strbuf_release(&contents);
103 - return -1;
101 + goto cleanup;
102 }
103 util->matching = -1;
104 in_header = 1;
@@ -111,11 +109,8 @@ static int read_patches(const char *range, struct string_list *list,
109 error(_("could not parse first line of `log` output: "
110 "did not start with 'commit ': '%s'"),
111 line);
114 - free(current_filename);
112 string_list_clear(list, 1);
116 - strbuf_release(&buf);
117 - strbuf_release(&contents);
118 - return -1;
113 + goto cleanup;
114 }
115
116 if (starts_with(line, "diff --git")) {
@@ -136,12 +131,9 @@ static int read_patches(const char *range, struct string_list *list,
131 if (len < 0) {
132 error(_("could not parse git header '%.*s'"),
133 orig_len, line);
139 - free(util);
140 - free(current_filename);
134 + FREE_AND_NULL(util);
135 string_list_clear(list, 1);
142 - strbuf_release(&buf);
143 - strbuf_release(&contents);
144 - return -1;
136 + goto cleanup;
137 }
138 strbuf_addstr(&buf, " ## ");
139 if (patch.is_new > 0)
@@ -219,6 +211,9 @@ static int read_patches(const char *range, struct string_list *list,
211 strbuf_addch(&buf, '\n');
212 util->diffsize++;
213 }
214 +
215 + ret = 0;
216 +cleanup:
217 strbuf_release(&contents);
218
219 if (util)
@@ -226,7 +221,7 @@ static int read_patches(const char *range, struct string_list *list,
221 strbuf_release(&buf);
222 free(current_filename);
223
229 - return 0;
224 + return ret;
225 }
226
227 static int patch_util_cmp(const void *dummy, const struct patch_util *a,