Re: [PATCH v4 08/11] reflog expire: don't lock reflogs using previously seen OID
From: Junio C Hamano <hidden>
Date: 2021-08-02 17:26:43
Ævar Arnfjörð Bjarmason [off-list ref] writes:
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.
quoted hunk
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.
quoted hunk
@@ -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.