On Thu, Apr 23, 2015 at 11:13:32AM -0700, Stefan Beller wrote:
> On Wed, Apr 22, 2015 at 4:24 PM, brian m. carlson
> <sandals@xxxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxx>
>
> 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:
> > @@ -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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html