Re: [PATCH v4 08/11] reflog expire: don't lock reflogs using previously seen OID
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 2, 2021, 17:26 UTC
- Message-ID
- <xmqqeebbwyeo.fsf@gitster.g>
- In-Reply-To
- <patch-08.11-c9c2da3599-20210726T234237Z-avarab@gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
Show 8 quoted lines
> During reflog expiry, the cmd_reflog_expire() function first iterates > ... > I'm leaving behind now-unused code the refs API etc. that takes the > now-NULL "unused_oid" argument, and other code that can be simplified now > that we never have on OID in that context, that'll be cleaned up in > subsequent commits, but for now let's narrowly focus on fixing the > "git gc" issue. As the modified assert() shows we always pass a NULL > oid to reflog_expire() now.
OK. Nicely described.
Show 28 quoted lines
> static int files_reflog_expire(struct ref_store *ref_store,
> - const char *refname, const struct object_id *oid,
> + const char *refname, const struct object_id *unused_oid,
> unsigned int flags,
> reflog_expiry_prepare_fn prepare_fn,
> reflog_expiry_should_prune_fn should_prune_fn,
> @@ -3049,6 +3049,7 @@ static int files_reflog_expire(struct ref_store *ref_store,
> int status = 0;
> int type;
> struct strbuf err = STRBUF_INIT;
> + const struct object_id *oid;
>
> memset(&cb, 0, sizeof(cb));
> cb.flags = flags;
> @@ -3060,7 +3061,7 @@ static int files_reflog_expire(struct ref_store *ref_store,
> * reference itself, plus we might need to update the
> * reference if --updateref was specified:
> */
> - lock = lock_ref_oid_basic(refs, refname, oid,
> + lock = lock_ref_oid_basic(refs, refname, NULL,
> REF_NO_DEREF,
> &type, &err);
> if (!lock) {
> @@ -3068,6 +3069,7 @@ static int files_reflog_expire(struct ref_store *ref_store,
> strbuf_release(&err);
> return -1;
> }
> + oid = &lock->old_oid;OK. That makes it more clear that the object name the locking code read is what gets used.
Show 8 quoted lines
> @@ -3111,6 +3113,7 @@ static int files_reflog_expire(struct ref_store *ref_store, > } > } > > + assert(!unused_oid); > (*prepare_fn)(refname, oid, cb.policy_cb); > refs_for_each_reflog_ent(ref_store, refname, expire_reflog_ent, &cb); > (*cleanup_fn)(cb.policy_cb);
The preference in this codebase is
ptr_to_function(params);
over
(*ptr_to_function)(params);
Once it is written and committed, it is not worth changing, but just for the record...
THanks.