Re: [PATCH v2 02/16] refs: convert for_each_tag_ref to struct object_id
On Thu, Apr 23, 2015 at 03:27:23PM -0400, Jeff King wrote:
> +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.
Sounds like a good idea. I'll reroll with that change probably sometime
this weekend.
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187
signature.asc
Description: Digital signature
Re: [PATCH v2 02/16] refs: convert for_each_tag_ref to struct object_id
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
> 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
>
> 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 02/16] refs: convert for_each_tag_ref to struct object_id
On Wed, Apr 22, 2015 at 4:24 PM, brian m. carlson
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
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
> ---
> builtin/pack-objects.c | 4 ++--
> builtin/rev-parse.c| 7 ++-
> builtin/tag.c | 8
> refs.c | 34 ++
> refs.h | 10 +-
> 5 files changed, 51 insertions(+), 12 deletions(-)
>
> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index c067107..0c69b0c 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -540,11 +540,11 @@ static enum write_one_status write_one(struct sha1file
> *f,
> return WRITE_ONE_WRITTEN;
> }
>
> -static int mark_tagged(const char *path, const unsigned char *sha1, int flag,
> +static int mark_tagged(const char *path, const struct object_id *oid, int
> flag,
>void *cb_data)
> {
> unsigned char peeled[20];
> - struct object_entry *entry = packlist_find(&to_pack, sha1, NULL);
> + struct object_entry *entry = packlist_find(&to_pack, oid->hash, NULL);
>
> if (entry)
> entry->tagged = 1;
> diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
> index 4d10dd9..7b70650 100644
> --- a/builtin/rev-parse.c
> +++ b/builtin/rev-parse.c
> @@ -198,6 +198,11 @@ static int show_reference(const char *refname, const
> unsigned char *sha1, int fl
> return 0;
> }
>
> +static int show_reference_oid(const char *refname, const struct object_id
> *oid, int flag, void *cb_data)
> +{
> + return show_reference(refname, oid->hash, flag, cb_data);
> +}
> +
> static int anti_reference(const char *refname, const unsigned char *sha1,
> int flag, void *cb_data)
> {
> show_rev(REVERSED, sha1, refname);
> @@ -682,7 +687,7 @@ int cmd_rev_parse(int argc, const char **argv, const char
> *prefix)
> continue;
> }
> if (!strcmp(arg, "--tags")) {
> - for_each_tag_ref(show_reference, NULL);
> + for_each_tag_ref(show_reference_oid, NULL);
> clear_ref_exclusion(&ref_excludes);
> continue;
> }
> diff --git a/builtin/tag.c b/builtin/tag.c
> index 6f07ac6..61399b7 100644
> --- a/builtin/tag.c
> +++ b/builtin/tag.c
> @@ -215,7 +215,7 @@ free_return:
> free(buf);
> }
>
> -static int show_reference(const char *refname, const unsigned char *sha1,
> +static int show_reference(const char *refname, const struct object_id *oid,
> int flag, void *cb_data)
> {
> struct tag_filter *filter = cb_data;
> @@ -224,14 +224,14 @@ static int show_reference(const char *refname, const
> unsigned char *sha1,
> if (filter->with_commit) {
> struct commit *commit;
>
> - commit = lookup_commit_reference_gently(sha1, 1);
> + commit = lookup_commit_reference_gently(oid->hash, 1);
> if (!commit)
> return 0;
> if (!contains(commit, filter->with_commit))
> return 0;
> }
>
> - if (points_at.nr && !match_points_at(refname, sha1))
> + if (points_at.nr && !match_points_at(refname, oid->hash))
> return 0;
>
> if (!filter->lines) {
> @@ -242,7 +242,7 @@ static int show_reference(const char *refname, const
> unsigned char *sha1,
> return 0;
> }
> printf("%-15s ", refname);
> - show_tag_lines(sha1, filter->lines);
> + show_tag_lines(oid->hash, filter->lines);
> putchar('\n');
> }
>
> diff --git a/refs.c b/refs.c
> index 522d15d..95863f2 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -694,6 +694,7 @@ struct ref_entry_cb {
> int trim;
> int flags;
> each_ref_fn *fn;
> + each_ref_fn_oid *fn_oid;
> void *cb_data;
> };
>
> @@ -717,8 +718,13 @@ static int do_one_ref(struct r
[PATCH v2 02/16] refs: convert for_each_tag_ref to struct object_id
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
---
builtin/pack-objects.c | 4 ++--
builtin/rev-parse.c| 7 ++-
builtin/tag.c | 8
refs.c | 34 ++
refs.h | 10 +-
5 files changed, 51 insertions(+), 12 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index c067107..0c69b0c 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -540,11 +540,11 @@ static enum write_one_status write_one(struct sha1file *f,
return WRITE_ONE_WRITTEN;
}
-static int mark_tagged(const char *path, const unsigned char *sha1, int flag,
+static int mark_tagged(const char *path, const struct object_id *oid, int flag,
void *cb_data)
{
unsigned char peeled[20];
- struct object_entry *entry = packlist_find(&to_pack, sha1, NULL);
+ struct object_entry *entry = packlist_find(&to_pack, oid->hash, NULL);
if (entry)
entry->tagged = 1;
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 4d10dd9..7b70650 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -198,6 +198,11 @@ static int show_reference(const char *refname, const
unsigned char *sha1, int fl
return 0;
}
+static int show_reference_oid(const char *refname, const struct object_id
*oid, int flag, void *cb_data)
+{
+ return show_reference(refname, oid->hash, flag, cb_data);
+}
+
static int anti_reference(const char *refname, const unsigned char *sha1, int
flag, void *cb_data)
{
show_rev(REVERSED, sha1, refname);
@@ -682,7 +687,7 @@ int cmd_rev_parse(int argc, const char **argv, const char
*prefix)
continue;
}
if (!strcmp(arg, "--tags")) {
- for_each_tag_ref(show_reference, NULL);
+ for_each_tag_ref(show_reference_oid, NULL);
clear_ref_exclusion(&ref_excludes);
continue;
}
diff --git a/builtin/tag.c b/builtin/tag.c
index 6f07ac6..61399b7 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -215,7 +215,7 @@ free_return:
free(buf);
}
-static int show_reference(const char *refname, const unsigned char *sha1,
+static int show_reference(const char *refname, const struct object_id *oid,
int flag, void *cb_data)
{
struct tag_filter *filter = cb_data;
@@ -224,14 +224,14 @@ static int show_reference(const char *refname, const
unsigned char *sha1,
if (filter->with_commit) {
struct commit *commit;
- commit = lookup_commit_reference_gently(sha1, 1);
+ commit = lookup_commit_reference_gently(oid->hash, 1);
if (!commit)
return 0;
if (!contains(commit, filter->with_commit))
return 0;
}
- if (points_at.nr && !match_points_at(refname, sha1))
+ if (points_at.nr && !match_points_at(refname, oid->hash))
return 0;
if (!filter->lines) {
@@ -242,7 +242,7 @@ static int show_reference(const char *refname, const
unsigned char *sha1,
return 0;
}
printf("%-15s ", refname);
- show_tag_lines(sha1, filter->lines);
+ show_tag_lines(oid->hash, filter->lines);
putchar('\n');
}
diff --git a/refs.c b/refs.c
index 522d15d..95863f2 100644
--- a/refs.c
+++ b/refs.c
@@ -694,6 +694,7 @@ struct ref_entry_cb {
int trim;
int flags;
each_ref_fn *fn;
+ each_ref_fn_oid *fn_oid;
void *cb_data;
};
@@ -717,8 +718,13 @@ static int do_one_ref(struct ref_entry *entry, void
*cb_data)
/* Store the old value, in case this is a recursive call: */
old_current_ref = current_ref;
current_ref = entry;
- retval = data->fn(entry->name + data->trim, entry->u.value.oid.hash,
- entry->flag, data->cb_data);
+ if (data->fn_oid) {
+ retval = data->fn_oid(entry->name + data->trim,
&entry->u.value.oid,
+entry->flag, data->cb_data);
+ } else {
+ retval = data->fn(entry->name + data->trim,
entry->u.value.
