René Scharfeprepare_revision_walk() allows callers to take ownership of the array of pending objects by setting the rev_info flag "leak_pending" and copying the object_array "pending". They use it to clear commit marks after setup is done. This interface is brittle enough that it requires extensive comments.
Provide an easier way by adding a function that can hand over the array to a caller-supplied output parameter and converting all users of the flag "leak_pending" to call prepare_revision_walk_extended() instead.
Signed-off-by: Rene Scharfe <l.s.r@web.de> --- Documentation/technical/api-revision-walking.txt | 6 ++++++ bisect.c | 17 +++++------------ builtin/checkout.c | 9 +-------- bundle.c | 9 +-------- revision.c | 10 +++++++++- revision.h | 14 ++------------ 6 files changed, 24 insertions(+), 41 deletions(-)
Show changes to 6 files +24 −41
Documentation/technical/api-revision-walking.txt, bisect.c, builtin/checkout.c, bundle.c, revision.c, revision.h
diff --git a/Documentation/technical/api-revision-walking.txt b/Documentation/technical/api-revision-walking.txt index 55b878ade8..9dc573d2ec 100644 --- a/Documentation/technical/api-revision-walking.txt +++ b/Documentation/technical/api-revision-walking.txt @@ -50,6 +50,12 @@ function. returns any error (non-zero return code) and if it does not, you can start using get_revision() to do the iteration. +`prepare_revision_walk_extended`:: + + Like prepare_revision_walk(), but allows callers to take ownership + of the array of pending objects by passing an object_array pointer + as the second parameter; passing NULL clears the array. + `get_revision`:: Takes a pointer to a `rev_info` structure and iterates over it, diff --git a/bisect.c b/bisect.c index 0fca17c02b..a2af405d28 100644 --- a/bisect.c +++ b/bisect.c @@ -641,9 +641,10 @@ static void bisect_rev_setup(struct rev_info *revs, const char *prefix, /* XXX leak rev_argv, as "revs" may still be pointing to it */ } -static void bisect_common(struct rev_info *revs) +static void bisect_common(struct rev_info *revs, + struct object_array *old_pending_ptr) { - if (prepare_revision_walk(revs)) + if (prepare_revision_walk_extended(revs, old_pending_ptr)) die("revision walk setup failed"); if (revs->tree_objects) mark_edges_uninteresting(revs, NULL); @@ -825,15 +826,7 @@ static int check_ancestors(const char *prefix) bisect_rev_setup(&revs, prefix, "^%s", "%s", 0); /* Save pending objects, so they can be cleaned up later. */ - pending_copy = revs.pending; - revs.leak_pending = 1; - - /* - * bisect_common calls prepare_revision_walk right away, which - * (together with .leak_pending = 1) makes us the sole owner of - * the list of pending objects. - */ - bisect_common(&revs); + bisect_common(&revs, &pending_copy); res = (revs.commits != NULL); /* Clean up objects used, as they will be reused. */ @@ -964,7 +957,7 @@ int bisect_next_all(const char *prefix, int no_checkout) bisect_rev_setup(&revs, prefix, "%s", "^%s", 1); revs.limited = 1; - bisect_common(&revs); + bisect_common(&revs, NULL); find_bisection(&revs.commits, &reaches, &all, !!skipped_revs.nr); revs.commits = managed_skipped(revs.commits, &tried); diff --git a/builtin/checkout.c b/builtin/checkout.c index e1e157d205..1f04f5d5e5 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -796,14 +796,7 @@ static void orphaned_commit_warning(struct commit *old, struct commit *new) add_pending_oid(&revs, "HEAD", &new->object.oid, UNINTERESTING); /* Save pending objects, so they can be cleaned up later. */ - refs = revs.pending; - revs.leak_pending = 1; - - /* - * prepare_revision_walk (together with .leak_pending = 1) makes us - * the sole owner of the list of pending objects. - */ - if (prepare_revision_walk(&revs)) + if (prepare_revision_walk_extended(&revs, &refs)) die(_("internal error in revision walk")); if (!(old->object.flags & UNINTERESTING)) suggest_reattach(old, &revs); diff --git a/bundle.c b/bundle.c index 93290962c9..6af6e38c40 100644 --- a/bundle.c +++ b/bundle.c @@ -158,14 +158,7 @@ int verify_bundle(struct bundle_header *header, int verbose) setup_revisions(2, argv, &revs, NULL); /* Save pending objects, so they can be cleaned up later. */ - refs = revs.pending; - revs.leak_pending = 1; - - /* - * prepare_revision_walk (together with .leak_pending = 1) makes us - * the sole owner of the list of pending objects. - */ - if (prepare_revision_walk(&revs)) + if (prepare_revision_walk_extended(&revs, &refs)) die(_("revision walk setup failed")); i = req_nr; diff --git a/revision.c b/revision.c index f6a3da5cd9..3a231451a4 100644 --- a/revision.c +++ b/revision.c @@ -2841,6 +2841,12 @@ void reset_revision_walk(void) } int prepare_revision_walk(struct rev_info *revs) +{ + return prepare_revision_walk_extended(revs, NULL); +} + +int prepare_revision_walk_extended(struct rev_info *revs, + struct object_array *old_pending_ptr) { int i; struct object_array old_pending; @@ -2860,7 +2866,9 @@ int prepare_revision_walk(struct rev_info *revs) } } } - if (!revs->leak_pending) + if (old_pending_ptr) + *old_pending_ptr = old_pending; + else object_array_clear(&old_pending); /* Signal whether we need per-parent treesame decoration */ diff --git a/revision.h b/revision.h index 54761200ad..5d4b475334 100644 --- a/revision.h +++ b/revision.h @@ -150,18 +150,6 @@ struct rev_info { date_mode_explicit:1, preserve_subject:1; unsigned int disable_stdin:1; - /* - * Set `leak_pending` to prevent `prepare_revision_walk()` from clearing - * the array of pending objects (`pending`). It will still forget about - * the array and its entries, so they really are leaked. This can be - * useful if the `struct object_array` `pending` is copied before - * calling `prepare_revision_walk()`. By setting `leak_pending`, you - * effectively claim ownership of the old array, so you should most - * likely call `object_array_clear(&pending_copy)` once you are done. - * Observe that this is about ownership of the array and its entries, - * not the commits referenced by those entries. - */ - unsigned int leak_pending:1; /* --show-linear-break */ unsigned int track_linear:1, track_first_time:1, @@ -270,6 +258,8 @@ extern int handle_revision_arg(const char *arg, struct rev_info *revs, extern void reset_revision_walk(void); extern int prepare_revision_walk(struct rev_info *revs); +extern int prepare_revision_walk_extended(struct rev_info *revs, + struct object_array *old_pending_ptr); extern struct commit *get_revision(struct rev_info *revs); extern char *get_revision_mark(const struct rev_info *revs, const struct commit *commit);
-- 2.15.1
