Subject: Re: [PATCH v4 11/32] delete_ref_loose(): don't muck around in the lock_file's filename

From: Johannes Sixt <[email-masked]>
Date: Sat, 13 Sep 2014 09:41:18 +0200

Am 06.09.2014 um 09:50 schrieb Michael Haggerty:
> It's bad manners.  Especially since, if unlink_or_warn() failed, the
> memory wasn't restored to its original contents.

I do not see how the old code did not restore the file name. Except for
this nit, the patch looks good.

> 
> So make our own copy to work with.
> 
> Signed-off-by: Michael Haggerty <[email-masked]>
> ---
>  refs.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/refs.c b/refs.c
> index 828522d..8a63073 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2545,12 +2545,15 @@ static int repack_without_ref(const char *refname)
>  static int delete_ref_loose(struct ref_lock *lock, int flag)
>  {
>  	if (!(flag & REF_ISPACKED) || flag & REF_ISSYMREF) {
> -		/* loose */
> -		int err, i = strlen(lock->lk->filename) - LOCK_SUFFIX_LEN;
> -
> -		lock->lk->filename[i] = 0;
> -		err = unlink_or_warn(lock->lk->filename);
> -		lock->lk->filename[i] = LOCK_SUFFIX[0];
> +		/*
> +		 * loose.  The loose file name is the same as the
> +		 * lockfile name, minus ".lock":
> +		 */
> +		char *loose_filename = xmemdupz(
> +				lock->lk->filename,
> +				strlen(lock->lk->filename) - LOCK_SUFFIX_LEN);
> +		int err = unlink_or_warn(loose_filename);
> +		free(loose_filename);
>  		if (err && errno != ENOENT)
>  			return 1;
>  	}
>