apply: do not leak patches and fragments

In the while loop inside apply_patch, patch and fragments are dynamically allocated with a calloc. However, only unused patches are actually freed and the rest are left to leak. Since a list is actively built up consisting of the used patches, they can simply be iterated and freed at the end of the function. In addition, the text in fragments were not freed, primarily because they mostly point into a patch text that is freed as a whole. But there are some cases where new piece of memory is allocated and pointed by a fragment (namely, when handling a binary patch). Introduce a free_patch bitfield to mark each fragment if its text needs to be freed, and free patches, fragments and fragment text that need to be freed when we are done with the input. Signed-off-by: Jared Hance <jaredhance@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jared Hance committed Mar 7, 2012 at 17:21 UTC 6fe53908f954b5683ca08d3745be1d76dd665483
1 file changed +23 -4
builtin/apply.c
+23 -4
@@ -152,8 +152,9 @@ struct fragment {
152 unsigned long oldpos, oldlines;
153 unsigned long newpos, newlines;
154 const char *patch;
155 + unsigned free_patch:1,
156 + rejected:1;
157 int size;
156 - int rejected;
158 int linenr;
159 struct fragment *next;
160 };
@@ -195,6 +196,24 @@ struct patch {
196 struct patch *next;
197 };
198
199 +static void free_patch(struct patch *patch)
200 +{
201 + while (patch) {
202 + struct patch *patch_next = patch->next;
203 + struct fragment *fragment = patch->fragments;
204 +
205 + while (fragment) {
206 + struct fragment *fragment_next = fragment->next;
207 + if (fragment->patch != NULL && fragment->free_patch)
208 + free((char *)fragment->patch);
209 + free(fragment);
210 + fragment = fragment_next;
211 + }
212 + free(patch);
213 + patch = patch_next;
214 + }
215 +}
216 +
217 /*
218 * A line in a file, len-bytes long (includes the terminating LF,
219 * except for an incomplete line at the end if the file ends with
@@ -1741,6 +1760,7 @@ static struct fragment *parse_binary_hunk(char **buf_p,
1760
1761 frag = xcalloc(1, sizeof(*frag));
1762 frag->patch = inflate_it(data, hunk_size, origlen);
1763 + frag->free_patch = 1;
1764 if (!frag->patch)
1765 goto corrupt;
1766 free(data);
@@ -3686,7 +3706,6 @@ static int apply_patch(int fd, const char *filename, int options)
3706 struct patch *list = NULL, **listp = &list;
3707 int skipped_patch = 0;
3708
3689 - /* FIXME - memory leak when using multiple patch files as inputs */
3709 memset(&fn_table, 0, sizeof(struct string_list));
3710 patch_input_file = filename;
3711 read_patch_file(&buf, fd);
@@ -3711,8 +3730,7 @@ static int apply_patch(int fd, const char *filename, int options)
3730 listp = &patch->next;
3731 }
3732 else {
3714 - /* perhaps free it a bit better? */
3715 - free(patch);
3733 + free_patch(patch);
3734 skipped_patch++;
3735 }
3736 offset += nr;
@@ -3753,6 +3771,7 @@ static int apply_patch(int fd, const char *filename, int options)
3771 if (summary)
3772 summary_patch_list(list);
3773
3774 + free_patch(list);
3775 strbuf_release(&buf);
3776 return 0;
3777 }