[PATCH] apply: plug strbuf leak

[PATCH] apply: plug strbuf leak

[Junio C Hamano]

Depending on how read_patch_file() fails, it may already have read
many bytes into the supplied strbuf.  Either the caller or the callee
should release the strbuf.

Here we choose to make the sole caller of the function responsible
for releasing it, as it makes the error handling slightly simpler.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 apply.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/apply.c b/apply.c
index 4aa1694cfa..0167902325 100644
--- a/apply.c
+++ b/apply.c
@@ -4881,8 +4881,10 @@ static int apply_patch(struct apply_state *state,
 
 	state->patch_input_file = filename;
 	state->linenr = 1;
-	if (read_patch_file(&buf, fd) < 0)
+	if (read_patch_file(&buf, fd) < 0) {
+		strbuf_release(&buf);
 		return -128;
+	}
 	offset = 0;
 	while (offset < buf.len) {
 		struct patch *patch;
-- 
2.54.0-398-ga4b2d32071

Re: [PATCH] apply: plug strbuf leak

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Depending on how read_patch_file() fails, it may already have read
> many bytes into the supplied strbuf.  Either the caller or the callee
> should release the strbuf.
>
> Here we choose to make the sole caller of the function responsible
> for releasing it, as it makes the error handling slightly simpler.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  apply.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/apply.c b/apply.c
> index 4aa1694cfa..0167902325 100644
> --- a/apply.c
> +++ b/apply.c
> @@ -4881,8 +4881,10 @@ static int apply_patch(struct apply_state *state,
>  
>  	state->patch_input_file = filename;
>  	state->linenr = 1;
> -	if (read_patch_file(&buf, fd) < 0)
> +	if (read_patch_file(&buf, fd) < 0) {
> +		strbuf_release(&buf);
>  		return -128;
> +	}

Ah, my mistake.  This was one of the two "oh, we found longstanding
issues immediately after enabling EXPENSIVE tests on" fixes Peff
already fixed for us.

>  	offset = 0;
>  	while (offset < buf.len) {
>  		struct patch *patch;