Re: [PATCH/RFC 4/7] refs: factor delete_ref loose ref step into a helper
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 29, 2013, 17:28 UTC
- Message-ID
- <xmqqppsw5rv8.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <f5a6f3552b16519b6568a7c745ab61c26bc4a072.1377784597.git.brad.king@kitware.com>
Brad King <brad.king@kitware.com> writes:
Show 29 quoted lines
> 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.
Show 23 quoted lines
> /* 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.