@matheusbsbs / matheus-bagliano / commits / f196c1e908

revisions API users: use release_revisions() needing REV_INFO_INIT

Use release_revisions() to various users of "struct rev_list" which need to have their "struct rev_info" zero-initialized before we can start using it. 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). For the submodule.c code see the "goto" on "(!left || !right || !sub)" added in 8e6df65015f (submodule: refactor show_submodule_summary with helper function, 2016-08-31). Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ævar Arnfjörð Bjarmason committed Apr 13, 2022 at 22:01 UTC f196c1e908db9242206e1fc9c64f33baf92676bb

5 files changed +44 -13

bisect.c

+12 -6

@@ -1010,7 +1010,7 @@ void read_bisect_terms(const char **read_bad, const char **read_good)

1010 */

1011 enum bisect_error bisect_next_all(struct repository *r, const char *prefix)

1012 {

1013 - struct rev_info revs;

1013 + struct rev_info revs = REV_INFO_INIT;

1014 struct commit_list *tried;

1015 int reaches = 0, all = 0, nr, steps;

1016 enum bisect_error res = BISECT_OK;

@@ -1035,7 +1035,7 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix)

1035

1036 res = check_good_are_ancestors_of_bad(r, prefix, no_checkout);

1037 if (res)

1038 - return res;

1038 + goto cleanup;

1039

1040 bisect_rev_setup(r, &revs, prefix, "%s", "^%s", 1);

1041

@@ -1060,14 +1060,16 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix)

1060 term_good,

1061 term_bad);

1062

1063 - return BISECT_FAILED;

1063 + res = BISECT_FAILED;

1064 + goto cleanup;

1065 }

1066

1067 if (!all) {

1068 fprintf(stderr, _("No testable commit found.\n"

1069 "Maybe you started with bad path arguments?\n"));

1070

1070 - return BISECT_NO_TESTABLE_COMMIT;

1071 + res = BISECT_NO_TESTABLE_COMMIT;

1072 + goto cleanup;

1073 }

1074

1075 bisect_rev = &revs.commits->item->object.oid;

@@ -1087,7 +1089,8 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix)

1089 * for negative return values for early returns up

1090 * until the cmd_bisect__helper() caller.

1091 */

1090 - return BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND;

1092 + res = BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND;

1093 + goto cleanup;

1094 }

1095

1096 nr = all - reaches - 1;

@@ -1106,7 +1109,10 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix)

1109 /* Clean up objects used, as they will be reused. */

1110 repo_clear_commit_marks(r, ALL_REV_FLAGS);

1111

1109 - return bisect_checkout(bisect_rev, no_checkout);

1112 + res = bisect_checkout(bisect_rev, no_checkout);

1113 +cleanup:

1114 + release_revisions(&revs);

1115 + return res;

1116 }

1117

1118 static inline int log2i(int n)

builtin/submodule--helper.c

+2 -1

@@ -766,7 +766,7 @@ static void status_submodule(const char *path, const struct object_id *ce_oid,

766 {

767 char *displaypath;

768 struct strvec diff_files_args = STRVEC_INIT;

769 - struct rev_info rev;

769 + struct rev_info rev = REV_INFO_INIT;

770 int diff_files_result;

771 struct strbuf buf = STRBUF_INIT;

772 const char *git_dir;

@@ -853,6 +853,7 @@ static void status_submodule(const char *path, const struct object_id *ce_oid,

853 cleanup:

854 strvec_clear(&diff_files_args);

855 free(displaypath);

856 + release_revisions(&rev);

857 }

858

859 static void status_submodule_cb(const struct cache_entry *list_item,

bundle.c

+8 -4

@@ -196,14 +196,16 @@ int verify_bundle(struct repository *r,

196 * to be verbose about the errors

197 */

198 struct string_list *p = &header->prerequisites;

199 - struct rev_info revs;

199 + struct rev_info revs = REV_INFO_INIT;

200 const char *argv[] = {NULL, "--all", NULL};

201 struct commit *commit;

202 int i, ret = 0, req_nr;

203 const char *message = _("Repository lacks these prerequisite commits:");

204

205 - if (!r || !r->objects || !r->objects->odb)

206 - return error(_("need a repository to verify a bundle"));

205 + if (!r || !r->objects || !r->objects->odb) {

206 + ret = error(_("need a repository to verify a bundle"));

207 + goto cleanup;

208 + }

209

210 repo_init_revisions(r, &revs, NULL);

211 for (i = 0; i < p->nr; i++) {

@@ -221,7 +223,7 @@ int verify_bundle(struct repository *r,

223 error("%s %s", oid_to_hex(oid), name);

224 }

225 if (revs.pending.nr != p->nr)

224 - return ret;

226 + goto cleanup;

227 req_nr = revs.pending.nr;

228 setup_revisions(2, argv, &revs, NULL);

229

@@ -284,6 +286,8 @@ int verify_bundle(struct repository *r,

286 printf_ln("The bundle uses this filter: %s",

287 list_objects_filter_spec(&header->filter));

288 }

289 +cleanup:

290 + release_revisions(&revs);

291 return ret;

292 }

293

revision.h

+20 -1

@@ -329,6 +329,25 @@ struct rev_info {

329 struct tmp_objdir *remerge_objdir;

330 };

331

332 +/**

333 + * Initialize the "struct rev_info" structure with a macro.

334 + *

335 + * This will not fully initialize a "struct rev_info", the

336 + * repo_init_revisions() function needs to be called before

337 + * setup_revisions() and any revision walking takes place.

338 + *

339 + * Use REV_INFO_INIT to make the "struct rev_info" safe for passing to

340 + * release_revisions() when it's inconvenient (e.g. due to a "goto

341 + * cleanup" pattern) to arrange for repo_init_revisions() to be called

342 + * before release_revisions() is called.

343 + *

344 + * Initializing with this REV_INFO_INIT is redundant to invoking

345 + * repo_init_revisions(). If repo_init_revisions() is guaranteed to be

346 + * called before release_revisions() the "struct rev_info" can be left

347 + * uninitialized.

348 + */

349 +#define REV_INFO_INIT { 0 }

350 +

351 /**

352 * Initialize a rev_info structure with default values. The third parameter may

353 * be NULL or can be prefix path, and then the `.prefix` variable will be set

@@ -363,7 +382,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs,

382

383 /**

384 * Free data allocated in a "struct rev_info" after it's been

366 - * initialized with repo_init_revisions().

385 + * initialized with repo_init_revisions() or REV_INFO_INIT.

386 */

387 void release_revisions(struct rev_info *revs);

388

submodule.c

+2 -1

@@ -638,7 +638,7 @@ void show_submodule_diff_summary(struct diff_options *o, const char *path,

638 struct object_id *one, struct object_id *two,

639 unsigned dirty_submodule)

640 {

641 - struct rev_info rev;

641 + struct rev_info rev = REV_INFO_INIT;

642 struct commit *left = NULL, *right = NULL;

643 struct commit_list *merge_bases = NULL;

644 struct repository *sub;

@@ -665,6 +665,7 @@ void show_submodule_diff_summary(struct diff_options *o, const char *path,

665

666 out:

667 free_commit_list(merge_bases);

668 + release_revisions(&rev);

669 clear_commit_marks(left, ~0);

670 clear_commit_marks(right, ~0);

671 if (sub) {