[PATCH] builtin/apply: free patch when parse_chunk() fails

[PATCH] builtin/apply: free patch when parse_chunk() fails

[Christian Couder]

When parse_chunk() fails it can return -1, for example
when find_header() doesn't find a patch header.

In this case it's better in apply_patch() to free the
"struct patch" that we just allocated instead of
leaking it.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/apply.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 42c610e..bf78282 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4373,8 +4373,10 @@ static int apply_patch(int fd, const char *filename, int options)
 		patch->inaccurate_eof = !!(options & INACCURATE_EOF);
 		patch->recount =  !!(options & RECOUNT);
 		nr = parse_chunk(buf.buf + offset, buf.len - offset, patch);
-		if (nr < 0)
+		if (nr < 0) {
+			free_patch(patch);
 			break;
+		}
 		if (apply_in_reverse)
 			reverse_patches(patch);
 		if (use_patch(patch)) {
-- 
2.8.0.rc2.54.g810e8ee

Re: [PATCH] builtin/apply: free patch when parse_chunk() fails

[Christian Couder]

On Wed, Mar 16, 2016 at 3:35 PM, Christian Couder
<christian.couder@gmail.com> wrote:
> When parse_chunk() fails it can return -1, for example
> when find_header() doesn't find a patch header.
>
> In this case it's better in apply_patch() to free the
> "struct patch" that we just allocated instead of
> leaking it.

Maybe this patch has fallen through the cracks too.

Re: [PATCH] builtin/apply: free patch when parse_chunk() fails

[Junio C Hamano]

Christian Couder <christian.couder@gmail.com> writes:

> On Wed, Mar 16, 2016 at 3:35 PM, Christian Couder
> <christian.couder@gmail.com> wrote:
>> When parse_chunk() fails it can return -1, for example
>> when find_header() doesn't find a patch header.
>>
>> In this case it's better in apply_patch() to free the
>> "struct patch" that we just allocated instead of
>> leaking it.
>
> Maybe this patch has fallen through the cracks too.

Anything worthy of discussion that you sent during the feature
freeze, please resend them to the list for discussion.

Re: [PATCH] builtin/apply: free patch when parse_chunk() fails

[Christian Couder]

On Fri, Apr 1, 2016 at 12:56 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Christian Couder <christian.couder@gmail.com> writes:
>
>> On Wed, Mar 16, 2016 at 3:35 PM, Christian Couder
>> <christian.couder@gmail.com> wrote:
>>> When parse_chunk() fails it can return -1, for example
>>> when find_header() doesn't find a patch header.
>>>
>>> In this case it's better in apply_patch() to free the
>>> "struct patch" that we just allocated instead of
>>> leaking it.
>>
>> Maybe this patch has fallen through the cracks too.
>
> Anything worthy of discussion that you sent during the feature
> freeze, please resend them to the list for discussion.

It looks like only the two patches I replied to have not been applied to next.
I had also sent a three patch long series
(http://thread.gmane.org/gmane.comp.version-control.git/289559), but
we agreed that only two of them could be merged for now and that's
what you did already (fda3e2c and db354b7).

By the way my guess is that replying to them is ok, but you ask to
"resend them", so I am wondering if you really prefer them to be
resent rather than just replied to.

Thanks!

Re: [PATCH] builtin/apply: free patch when parse_chunk() fails

[Junio C Hamano]

Christian Couder <christian.couder@gmail.com> writes:

> When parse_chunk() fails it can return -1, for example
> when find_header() doesn't find a patch header.
>
> In this case it's better in apply_patch() to free the
> "struct patch" that we just allocated instead of
> leaking it.
>
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
>  builtin/apply.c | 4 +++-

OK.  Will queue.  Thanks.

>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/apply.c b/builtin/apply.c
> index 42c610e..bf78282 100644
> --- a/builtin/apply.c
> +++ b/builtin/apply.c
> @@ -4373,8 +4373,10 @@ static int apply_patch(int fd, const char *filename, int options)
>  		patch->inaccurate_eof = !!(options & INACCURATE_EOF);
>  		patch->recount =  !!(options & RECOUNT);
>  		nr = parse_chunk(buf.buf + offset, buf.len - offset, patch);
> -		if (nr < 0)
> +		if (nr < 0) {
> +			free_patch(patch);
>  			break;
> +		}
>  		if (apply_in_reverse)
>  			reverse_patches(patch);
>  		if (use_patch(patch)) {