Re: [PATCH 2/2] lockfile: set lk->fd = -1 in 'rollback_lock_file()'

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 10, 2009, 15:29 UTC
Message-ID: <7v63jipax4.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <1234334612-24907-2-git-send-email-guanqun.lu@gmail.com>

Guanqun Lu <guanqun.lu@gmail.com> writes:

Show 8 quoted lines

> Post-conditions when these functions return successfully:
>                     lk->fd == -1?    lk->filename[0] == '\0'?
> close_lock_file()       yes                 no
> commit_lock_file()      yes                 yes
> rollback_lock_file()    no*                 yes
>
> [*] This commit changes this 'no' in rollback_lock_file() to 'yes',
> which achieves more robust and unified interface.

Is there a broken caller this patch fixes? IOW, is there a codepath that can call rollback and then later notices lk->fd is not negative and uses it without checking lk->filename[0] first?

Show 23 quoted lines

> Signed-off-by: Guanqun Lu <guanqun.lu@gmail.com>
> ---
>  lockfile.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/lockfile.c b/lockfile.c
> index 021c337..44e5253 100644
> --- a/lockfile.c
> +++ b/lockfile.c
> @@ -243,8 +243,10 @@ int commit_locked_index(struct lock_file *lk)
>  void rollback_lock_file(struct lock_file *lk)
>  {
>  	if (lk->filename[0]) {
> -		if (lk->fd >= 0)
> +		if (lk->fd >= 0) {
>  			close(lk->fd);
> +			lk->fd = -1;
> +		}
>  		unlink(lk->filename);
>  	}
>  	lk->filename[0] = 0;
> -- 
> 1.6.1.2.392.gb04d1