threads / patch / 41714

patchbuiltin/apply: free patch when parse_chunk() fails

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

## tl;dr

5 messages between Mar 16, 2016 and Apr 1, 2016. Diffs are folded; open one to read it.

replies: 4people: 2as markdown or json

Christian Couder· Mar 16, 2016, 19:35 UTC · lore

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(-)
Show changes to builtin/apply.c +3 −1
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
Christian Couder· Mar 31, 2016, 22:29 UTC · re: Christian Couder · lore

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

On Wed, Mar 16, 2016 at 3:35 PM, Christian Couder <christian.couder@gmail.com> wrote:

Show 6 quoted lines
> 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.
Junio C Hamano· Mar 31, 2016, 22:56 UTC · re: Christian Couder · lore

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

Christian Couder <christian.couder@gmail.com> writes:
Show 10 quoted lines
> 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.

Christian Couder· Mar 31, 2016, 23:58 UTC · re: Junio C Hamano · lore

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

On Fri, Apr 1, 2016 at 12:56 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 15 quoted lines
> 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!
Junio C Hamano· Apr 1, 2016, 17:23 UTC · re: Christian Couder · lore

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

Christian Couder <christian.couder@gmail.com> writes:
Show 10 quoted lines
> 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.
Show 18 quoted lines
>  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)) {

← back to recent threads