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

2013-08-29 Thread Brad King
On 08/29/2013 01:28 PM, Junio C Hamano wrote:
> Brad King  writes:
>> -if (!(flag & REF_ISPACKED) || flag & REF_ISSYMREF) {
>> +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.

Okay, I removed this part of the change.  It makes the commit
simpler anyway.

-Brad
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-08-29 Thread Junio C Hamano
Brad King  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 
> ---
>  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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html