Re: [PATCH 2/3] Fix memory leak in apply_patch in apply.c.

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 3, 2012, 07:41 UTC
Message-ID: <7v399q8732.fsf@alter.siamese.dyndns.org>
In-Reply-To: <03cc2ba05a5744d8b48700556f1942757151ec4b.1330740964.git.jaredhance@gmail.com>

Jared Hance <jaredhance@gmail.com> writes:

Show 6 quoted lines

> @@ -3712,7 +3712,6 @@ static int apply_patch(int fd, const char *filename, int options)
>  			listp = &patch->next;
>  		}
>  		else {
> -			/* perhaps free it a bit better? */
>  			free(patch);

This "free it better" comment is not about how to free the "struct patch" itself, but is about the piece of memory pointed at it, "struct fragment", and pieces of patch text pointed at them. The patch text pointed with frag->patch starts out as a location in buf.buf (which will be freed later in this function), but IIRC there were places deeper in the callchain that replace the pointer with allocated memory.

Show 9 quoted lines

>  			skipped_patch++;
>  		}
> @@ -3753,6 +3752,13 @@ static int apply_patch(int fd, const char *filename, int options)
>  
>  	if (summary)
>  		summary_patch_list(list);
> +	
> +	patch_iter = list;
> +	while(patch_iter != NULL) {

	while (patch_iter) {

Show 7 quoted lines

> +	    struct patch *patch_iter_next = patch_iter->next;
> +	    free(patch_iter);
> +	    patch_iter = patch_iter_next;
> +	}
>  
>  	strbuf_release(&buf);
>  	return 0;