git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2 02/16] refs: convert for_each_tag_ref to struct object_id

From
Jeff King <peff@peff.net>
Date
Apr 23, 2015, 19:27 UTC
Message-ID
<20150423192723.GA29220@peff.net>
In-Reply-To
<CAGZ79kas9QgJy1ooCjHPR+ZGGuTdMhTxO9zSMxnGpPk8rG5_xQ@mail.gmail.com>
On Thu, Apr 23, 2015 at 11:13:32AM -0700, Stefan Beller wrote:
Show 18 quoted lines
> On Wed, Apr 22, 2015 at 4:24 PM, brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
> > To allow piecemeal conversion of the for_each_*_ref functions, introduce
> > an additional typedef for a callback function that takes struct
> > object_id * instead of unsigned char *.  Provide an extra field in
> > struct ref_entry_cb for this callback and ensure at most one is set at a
> > time.  Temporarily suffix these new entries with _oid to distinguish
> > them.  Convert for_each_tag_ref and its callers to use the new _oid
> > functions, introducing temporary wrapper functions to avoid type
> > mismatches.
> >
> > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> 
> I am currently running this patch series via
> git rebase -i origin/next --exec=make --exec="make test"
> through the compilation and test suite one by one.
> (My current view of origin/next is (c8da2d582, Sync with 2.4.0-rc3)
> and this commit fails in t5312-prune-corruption.sh test 3 5 and 8
It's because of this hunk:
Show 22 quoted lines
> > @@ -1950,6 +1956,21 @@ static int do_for_each_ref(struct ref_cache *refs, const char *base,
> >         data.trim = trim;
> >         data.flags = flags;
> >         data.fn = fn;
> > +       data.fn_oid = NULL;
> > +       data.cb_data = cb_data;
> > +
> > +       return do_for_each_entry(refs, base, do_one_ref, &data);
> > +}
> > +
> > +static int do_for_each_ref_oid(struct ref_cache *refs, const char *base,
> > +                          each_ref_fn_oid fn, int trim, int flags, void *cb_data)
> > +{
> > +       struct ref_entry_cb data;
> > +       data.base = base;
> > +       data.trim = trim;
> > +       data.flags = flags;
> > +       data.fn = NULL;
> > +       data.fn_oid = fn;
> >         data.cb_data = cb_data;
> >
> >         if (ref_paranoia < 0)

The ref_paranoia code gets pushed down into do_for_each_ref_oid, but it needs called in both do_for_each_ref variants. This is probably an artifact of rebasing the patches (the ref_paranoia stuff was added recently).

I think it would make sense to pull the setup of the data struct into a shared function rather than duplicate it. But we want to avoid having to update do_for_each_ref callsites, so we'll have to provide a wrapper.

Like this:
diff --git a/refs.c b/refs.c
index 95863f2..ad39d74 100644
--- a/refs.c
+++ b/refs.c
@@ -1948,29 +1948,16 @@ static int do_for_each_entry(struct ref_cache *refs, const char *base,
  * value, stop the iteration and return that value; otherwise, return
  * 0.
  */
-static int do_for_each_ref(struct ref_cache *refs, const char *base,
-			   each_ref_fn fn, int trim, int flags, void *cb_data)
+static int do_for_each_ref_generic(struct ref_cache *refs, const char *base,
+				   each_ref_fn fn, each_ref_fn_oid fn_oid,
+				   int trim, int flags, void *cb_data)
 {
 	struct ref_entry_cb data;
 	data.base = base;
 	data.trim = trim;
 	data.flags = flags;
 	data.fn = fn;
-	data.fn_oid = NULL;
-	data.cb_data = cb_data;
-
-	return do_for_each_entry(refs, base, do_one_ref, &data);
-}
-
-static int do_for_each_ref_oid(struct ref_cache *refs, const char *base,
-			   each_ref_fn_oid fn, int trim, int flags, void *cb_data)
-{
-	struct ref_entry_cb data;
-	data.base = base;
-	data.trim = trim;
-	data.flags = flags;
-	data.fn = NULL;
-	data.fn_oid = fn;
+	data.fn_oid = fn_oid;
 	data.cb_data = cb_data;
 
 	if (ref_paranoia < 0)
@@ -1981,6 +1968,18 @@ static int do_for_each_ref_oid(struct ref_cache *refs, const char *base,
 	return do_for_each_entry(refs, base, do_one_ref, &data);
 }
 
+static int do_for_each_ref(struct ref_cache *refs, const char *base,
+			   each_ref_fn fn, int trim, int flags, void *cb_data)
+{
+	return do_for_each_ref_generic(refs, base, fn, NULL, trim, flags, cb_data);
+}
+
+static int do_for_each_ref_oid(struct ref_cache *refs, const char *base,
+			       each_ref_fn_oid fn, int trim, int flags, void *cb_data)
+{
+	return do_for_each_ref_generic(refs, base, NULL, fn, trim, flags, cb_data);
+}
+
 static int do_head_ref(const char *submodule, each_ref_fn fn, void *cb_data)
 {
 	unsigned char sha1[20];

You can even dispense with the _oid variant wrapper, and just call into
the generic version directly from the new callsites.

-Peff
Previous: Stefan BellerNext: brian m. carlson
Message 7 of 24 in “Convert parts of refs.c to struct object_id”
  1. 00/16 Convert parts of refs.c to struct object_idbrian m. carlson, Apr 22, 2015
  2. 01/16 refs: convert struct ref_entry to use struct object_idbrian m. carlson, Apr 22, 2015
  3. Stefan BellerApr 23, 2015
  4. brian m. carlsonApr 24, 2015
  5. 02/16 refs: convert for_each_tag_ref to struct object_idbrian m. carlson, Apr 22, 2015
  6. Stefan BellerApr 23, 2015
  7. Jeff KingApr 23, 2015
  8. brian m. carlsonApr 24, 2015
  9. 03/16 refs: convert remaining users of for_each_ref_in to object_idbrian m. carlson, Apr 22, 2015
  10. 04/16 refs: convert for_each_ref_in_submodule to object_idbrian m. carlson, Apr 22, 2015
  11. 05/16 refs: convert head_ref to struct object_idbrian m. carlson, Apr 22, 2015
  12. 06/16 refs: convert for_each_ref_submodule to struct object_idbrian m. carlson, Apr 22, 2015
  13. 07/16 revision: remove unused _oid helper.brian m. carlson, Apr 22, 2015
  14. 08/16 refs: convert for_each_ref to struct object_idbrian m. carlson, Apr 22, 2015
  15. 09/16 refs: convert for_each_replace_ref to struct object_idbrian m. carlson, Apr 22, 2015
  16. 10/16 refs: convert namespaced ref iteration functions to object_idbrian m. carlson, Apr 22, 2015
  17. 11/16 refs: convert for_each_rawref to struct object_id.brian m. carlson, Apr 22, 2015
  18. 12/16 refs: rename do_for_each_ref_oid to do_for_each_refbrian m. carlson, Apr 22, 2015
  19. 13/16 refs: convert for_each_reflog to struct object_idbrian m. carlson, Apr 22, 2015
  20. 14/16 refs: rename each_ref_fn_oid to each_ref_fnbrian m. carlson, Apr 22, 2015
  21. 15/16 Remove unneeded *_oid functions.brian m. carlson, Apr 22, 2015
  22. 16/16 refs: convert struct ref_lock to struct object_idbrian m. carlson, Apr 22, 2015
  23. Michael HaggertyApr 26, 2015
  24. brian m. carlsonMay 3, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.