Patrick Steinhardt <ps@xxxxxx> writes: > 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 ... > +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. > +/* > + * 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.