[PATCH v3 08/27] revisions API users: add "goto cleanup" for "rev_info" early exit
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Mar 25, 2022, 17:18 UTC
- Message-ID
- <patch-v3-08.27-f8a9443fe6f-20220325T171340Z-avarab@gmail.com>
- In-Reply-To
- <cover-v3-00.27-00000000000-20220325T171340Z-avarab@gmail.com>
Add release_revisions() in various users of "struct rev_info" that can mostly use a "goto cleanup" pattern, but also have an early "return" before we've called repo_init_revisions(). We need to avoid calling release_revisions() with uninitialized memory.
It would be a lot cleaner to be able to initialize "struct rev_info" with "{ 0 }" here, or if a "REV_INFO_INIT" existed, we'll hopefully get around to making the initialization easier in the future (now it can't be done via a macro).
Until then let's leave a "cleanup_no_rev[s]" in place to document the intention here. Only status_submodule() in builtin/submodule--helper.c strictly speaking needs this, the other ones could keep their "return" for the early exit. But let's have them also use the "goto cleanup[...]" for consistency, and for the eventual migration to simpler initialization.
For the bundle.c code see the early exit case added in 3bbbe467f29 (bundle verify: error out if called without an object database, 2019-05-27).
For the relevant bisect.c code see 45b6370812c (bisect: libify `check_good_are_ancestors_of_bad` and its dependents, 2020-02-17).
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- bisect.c | 17 ++++++++++++----- builtin/submodule--helper.c | 6 ++++-- bundle.c | 12 +++++++++--- 3 files changed, 25 insertions(+), 10 deletions(-)
diff --git a/bisect.c b/bisect.c index cc6b8b6230d..c2e9f6ca9f6 100644 --- a/bisect.c +++ b/bisect.c @@ -1035,7 +1035,7 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix) res = check_good_are_ancestors_of_bad(r, prefix, no_checkout); if (res) - return res; + goto cleanup_no_revs; bisect_rev_setup(r, &revs, prefix, "%s", "^%s", 1); @@ -1060,14 +1060,16 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix) term_good, term_bad); - return BISECT_FAILED; + res = BISECT_FAILED; + goto cleanup; } if (!all) { fprintf(stderr, _("No testable commit found.\n" "Maybe you started with bad path arguments?\n")); - return BISECT_NO_TESTABLE_COMMIT; + res = BISECT_NO_TESTABLE_COMMIT; + goto cleanup; } bisect_rev = &revs.commits->item->object.oid; @@ -1087,7 +1089,8 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix) * for negative return values for early returns up * until the cmd_bisect__helper() caller. */ - return BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND; + res = BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND; + goto cleanup; } nr = all - reaches - 1; @@ -1106,7 +1109,11 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix) /* Clean up objects used, as they will be reused. */ repo_clear_commit_marks(r, ALL_REV_FLAGS); - return bisect_checkout(bisect_rev, no_checkout); + res = bisect_checkout(bisect_rev, no_checkout); +cleanup: + release_revisions(&revs); +cleanup_no_revs: + return res; } static inline int log2i(int n) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 24980863f68..d1b656c0909 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -779,7 +779,7 @@ static void status_submodule(const char *path, const struct object_id *ce_oid, if ((CE_STAGEMASK & ce_flags) >> CE_STAGESHIFT) { print_status(flags, 'U', path, null_oid(), displaypath); - goto cleanup; + goto cleanup_no_rev; } strbuf_addf(&buf, "%s/.git", path); @@ -791,7 +791,7 @@ static void status_submodule(const char *path, const struct object_id *ce_oid, !is_git_directory(git_dir)) { print_status(flags, '-', path, ce_oid, displaypath); strbuf_release(&buf); - goto cleanup; + goto cleanup_no_rev; } strbuf_release(&buf); @@ -851,6 +851,8 @@ static void status_submodule(const char *path, const struct object_id *ce_oid, } cleanup: + release_revisions(&rev); +cleanup_no_rev: strvec_clear(&diff_files_args); free(displaypath); } diff --git a/bundle.c b/bundle.c index e359370cfcd..9b7c0bc55c4 100644 --- a/bundle.c +++ b/bundle.c @@ -202,8 +202,10 @@ int verify_bundle(struct repository *r, int i, ret = 0, req_nr; const char *message = _("Repository lacks these prerequisite commits:"); - if (!r || !r->objects || !r->objects->odb) - return error(_("need a repository to verify a bundle")); + if (!r || !r->objects || !r->objects->odb) { + ret = error(_("need a repository to verify a bundle")); + goto cleanup_no_revs; + } repo_init_revisions(r, &revs, NULL); for (i = 0; i < p->nr; i++) { @@ -221,7 +223,7 @@ int verify_bundle(struct repository *r, error("%s %s", oid_to_hex(oid), name); } if (revs.pending.nr != p->nr) - return ret; + goto cleanup; req_nr = revs.pending.nr; setup_revisions(2, argv, &revs, NULL); @@ -283,6 +285,10 @@ int verify_bundle(struct repository *r, list_refs(r, 0, NULL); } } + +cleanup: + release_revisions(&revs); +cleanup_no_revs: return ret; }
-- 2.35.1.1509.ge4eeb5bd39e