Re: [PATCH 4/6] refs: drop unused params from the reflog iterator callback
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 20, 2024, 00:14 UTC
- Message-ID
- <xmqqplwsyotj.fsf@gitster.g>
- In-Reply-To
- <be512ef268b910852ff11df181d89c483ffc18ab.1708353264.git.ps@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
Show 9 quoted lines
> The ref and reflog iterators share much of the same underlying code to > iterate over the corresponding entries. This results in some weird code > because the reflog iterator also exposes an object ID as well as a flag > to the callback function. Neither of these fields do refer to the reflog > though -- they refer to the corresponding ref with the same name. This > is quite misleading. In practice at least the object ID cannot really be > implemented in any other way as a reflog does not have a specific object > ID in the first place. This is further stressed by the fact that none of > the callbacks except for our test helper make use of these fields.
Interesting observation. Of course this will make the callstack longer by another level of indirection ...
Show 14 quoted lines
> +struct do_for_each_reflog_help {
> + each_reflog_fn *fn;
> + void *cb_data;
> +};
> +
> +static int do_for_each_reflog_helper(struct repository *r UNUSED,
> + const char *refname,
> + const struct object_id *oid UNUSED,
> + int flags,
> + void *cb_data)
> +{
> + struct do_for_each_reflog_help *hp = cb_data;
> + return hp->fn(refname, hp->cb_data);
> +}... but I think it would be worth it.
Show 15 quoted lines
> +/*
> + * The signature for the callback function for the {refs_,}for_each_reflog()
> + * functions below. The memory pointed to by the refname argument is only
> + * guaranteed to be valid for the duration of a single callback invocation.
> + */
> +typedef int each_reflog_fn(const char *refname, void *cb_data);
> +
> /*
> * Calls the specified function for each reflog file until it returns nonzero,
> * and returns the value. Reflog file order is unspecified.
> */
> -int refs_for_each_reflog(struct ref_store *refs, each_ref_fn fn, void *cb_data);
> -int for_each_reflog(each_ref_fn fn, void *cb_data);
> +int refs_for_each_reflog(struct ref_store *refs, each_reflog_fn fn, void *cb_data);
> +int for_each_reflog(each_reflog_fn fn, void *cb_data);Nice simplification.