Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> 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.
> 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.
> @@ -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.
