Subject: Re: [PATCH 2/2] config.c: handle lock file in error case in git_config_rename_...

From: Junio C Hamano <[email-masked]>
Date: Tue, 20 Dec 2016 12:09:58 -0800

Nguyễn Thái Ngọc Duy  <[email-masked]> writes:

> We could rely on atexit() to clean up everything, but let's be
> explicit when we can. And it's good anyway because the function is
> called the second time in the same process, we're in trouble.
>
> This function should not affect the successful case because after
> commit_lock_file() is called, rollback_lock_file() becomes no-op.

Not really.  At the point of the first "goto out" in this function,
lock is merely an uninitialized pointer.

>
> Signed-off-by: Nguyễn Thái Ngọc Duy <[email-masked]>
> ---

>  config.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/config.c b/config.c
> index 505e0d0..e02def4 100644
> --- a/config.c
> +++ b/config.c
> @@ -2483,6 +2483,7 @@ int git_config_rename_section_in_file(const char *config_filename,
>  		ret = error_errno("could not write config file %s",
>  				  config_filename);
>  out:
> +	rollback_lock_file(lock);
>  	free(filename_buf);
>  	return ret;
>  }