Re: [PATCH v2 1/1] maintenance: fix two memory leaks

[Derrick Stolee <stolee@gmail.com>]

On 5/10/2021 3:59 PM, Lénaïc Huard wrote:
> Fixes two memory leaks when running `git maintenance start` or `git
> maintenance stop` in `update_background_schedule`:

Thanks for finding these leaks.

> ---
>  builtin/gc.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/builtin/gc.c b/builtin/gc.c
> index ef7226d7bc..484fe983d3 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -1924,6 +1924,7 @@ static int crontab_update_schedule(int run_maintenance, int fd, const char *cmd)
>  		else if (!in_old_region)
>  			fprintf(cron_in, "%s\n", line.buf);
>  	}
> +	strbuf_release(&line);
>  
>  	if (run_maintenance) {
>  		struct strbuf line_format = STRBUF_INIT;
> @@ -1986,8 +1987,10 @@ static int update_background_schedule(int enable)
>  		cmd = sep + 1;
>  	}
>  
> -	if (hold_lock_file_for_update(&lk, lock_path, LOCK_NO_DEREF) < 0)
> -		return error(_("another process is scheduling background maintenance"));
> +	if (hold_lock_file_for_update(&lk, lock_path, LOCK_NO_DEREF) < 0) {
> +		result = error(_("another process is scheduling background maintenance"));
> +		goto cleanup;
> +	}
>  
>  	if (!strcmp(scheduler, "launchctl"))
>  		result = launchctl_update_schedule(enable, get_lock_file_fd(&lk), cmd);
> @@ -1999,6 +2002,9 @@ static int update_background_schedule(int enable)
>  		die("unknown background scheduler: %s", scheduler);
>  
>  	rollback_lock_file(&lk);
> +
> +cleanup:
> +	free(lock_path);

And I agree that this version looks good. 

Thanks,
-Stolee