Re: [PATCH/RFC 4/7] refs: factor delete_ref loose ref step into a helper

[Junio C Hamano <gitster@pobox.com>]

Brad King <brad.king@kitware.com> writes:

> Factor loose ref deletion into helper function delete_ref_loose to allow
> later use elsewhere.  While at it, rename local names 'flag => type' and
> 'delopt => flags' for consistency with callers and called functions.
>
> Signed-off-by: Brad King <brad.king@kitware.com>
> ---
>  refs.c |   24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index 2e755b4..5908648 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2450,15 +2450,10 @@ static int repack_without_ref(const char *refname)
>  	return commit_packed_refs();
>  }
>  
> -int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
> +static int delete_ref_loose(struct ref_lock *lock, int type)
>  {
> -	struct ref_lock *lock;
> -	int err, i = 0, ret = 0, flag = 0;
> -
> -	lock = lock_ref_sha1_basic(refname, sha1, delopt, &flag);
> -	if (!lock)
> -		return 1;
> -	if (!(flag & REF_ISPACKED) || flag & REF_ISSYMREF) {
> +	int err, i, ret = 0;
> +	if (!(type & REF_ISPACKED) || type & REF_ISSYMREF) {

Hits from "git grep REF_IS" tell me that all users of REF_IS* symbol
that check if a bit is on in a word does so against "flag" (either a
variable called "flag", "flags", or a structure member ".flag").

This change is making things less consistent, not more, I am afraid.

>  		/* loose */
>  		i = strlen(lock->lk->filename) - 5; /* .lock */
>  		lock->lk->filename[i] = 0;
> @@ -2468,6 +2463,19 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
>  
>  		lock->lk->filename[i] = '.';
>  	}
> +	return ret;
> +}
> +
> +int delete_ref(const char *refname, const unsigned char *sha1, int flags)
> +{
> +	struct ref_lock *lock;
> +	int ret = 0, type = 0;
> +
> +	lock = lock_ref_sha1_basic(refname, sha1, flags, &type);
> +	if (!lock)
> +		return 1;
> +	ret |= delete_ref_loose(lock, type);
> +
>  	/* removing the loose one could have resurrected an earlier
>  	 * packed one.  Also, if it was not loose we need to repack
>  	 * without it.