freshlybakedca.ke / git
Git fork
fork atom

commit-reach: use `size_t` to track indices when computing merge bases

The functions `repo_get_merge_bases_many()` and friends accepts an array
of commits as well as a parameter that indicates how large that array
is. This parameter is using a signed integer, which leads to a couple of
warnings with -Wsign-compare.

Refactor the code to use `size_t` to track indices instead and adapt
callers accordingly. While most callers are trivial, there are two
callers that require a bit more scrutiny:

- builtin/merge-base.c:show_merge_base() subtracts `1` from the
`rev_nr` before calling `repo_get_merge_bases_many_dirty()`, so if
the variable was `0` it would wrap. This code is fine though because
its only caller will execute that code only when `argc >= 2`, and it
follows that `rev_nr >= 2`, as well.

- bisect.ccheck_merge_bases() similarly subtracts `1` from `rev_nr`.
Again, there is only a single caller that populates `rev_nr` with
`good_revs.nr`. And because a bisection always requires at least one
good revision it follws that `rev_nr >= 1`.

Mark the file as -Wsign-compare-clean.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

authored by

Patrick Steinhardt and committed by
Junio C Hamano 5e7fe8a7 455ac070

+11 -12
unified split
+1 -1
bisect.c
··· 855 * for early success, this will be converted back to 0 in 856 * check_good_are_ancestors_of_bad(). 857 */ 858 - static enum bisect_error check_merge_bases(int rev_nr, struct commit **rev, int no_checkout) 859 { 860 enum bisect_error res = BISECT_OK; 861 struct commit_list *result = NULL;
··· 855 * for early success, this will be converted back to 0 in 856 * check_good_are_ancestors_of_bad(). 857 */ 858 + static enum bisect_error check_merge_bases(size_t rev_nr, struct commit **rev, int no_checkout) 859 { 860 enum bisect_error res = BISECT_OK; 861 struct commit_list *result = NULL;
+2 -2
builtin/merge-base.c
··· 8 #include "parse-options.h" 9 #include "commit-reach.h" 10 11 - static int show_merge_base(struct commit **rev, int rev_nr, int show_all) 12 { 13 struct commit_list *result = NULL, *r; 14 ··· 149 struct repository *repo UNUSED) 150 { 151 struct commit **rev; 152 - int rev_nr = 0; 153 int show_all = 0; 154 int cmdmode = 0; 155 int ret;
··· 8 #include "parse-options.h" 9 #include "commit-reach.h" 10 11 + static int show_merge_base(struct commit **rev, size_t rev_nr, int show_all) 12 { 13 struct commit_list *result = NULL, *r; 14 ··· 149 struct repository *repo UNUSED) 150 { 151 struct commit **rev; 152 + size_t rev_nr = 0; 153 int show_all = 0; 154 int cmdmode = 0; 155 int ret;
+3 -4
commit-reach.c
··· 1 #define USE_THE_REPOSITORY_VARIABLE 2 - #define DISABLE_SIGN_COMPARE_WARNINGS 3 4 #include "git-compat-util.h" 5 #include "commit.h" ··· 421 422 static int get_merge_bases_many_0(struct repository *r, 423 struct commit *one, 424 - int n, 425 struct commit **twos, 426 int cleanup, 427 struct commit_list **result) ··· 469 470 int repo_get_merge_bases_many(struct repository *r, 471 struct commit *one, 472 - int n, 473 struct commit **twos, 474 struct commit_list **result) 475 { ··· 478 479 int repo_get_merge_bases_many_dirty(struct repository *r, 480 struct commit *one, 481 - int n, 482 struct commit **twos, 483 struct commit_list **result) 484 {
··· 1 #define USE_THE_REPOSITORY_VARIABLE 2 3 #include "git-compat-util.h" 4 #include "commit.h" ··· 420 421 static int get_merge_bases_many_0(struct repository *r, 422 struct commit *one, 423 + size_t n, 424 struct commit **twos, 425 int cleanup, 426 struct commit_list **result) ··· 468 469 int repo_get_merge_bases_many(struct repository *r, 470 struct commit *one, 471 + size_t n, 472 struct commit **twos, 473 struct commit_list **result) 474 { ··· 477 478 int repo_get_merge_bases_many_dirty(struct repository *r, 479 struct commit *one, 480 + size_t n, 481 struct commit **twos, 482 struct commit_list **result) 483 {
+2 -2
commit-reach.h
··· 14 struct commit *rev2, 15 struct commit_list **result); 16 int repo_get_merge_bases_many(struct repository *r, 17 - struct commit *one, int n, 18 struct commit **twos, 19 struct commit_list **result); 20 /* To be used only when object flags after this call no longer matter */ 21 int repo_get_merge_bases_many_dirty(struct repository *r, 22 - struct commit *one, int n, 23 struct commit **twos, 24 struct commit_list **result); 25
··· 14 struct commit *rev2, 15 struct commit_list **result); 16 int repo_get_merge_bases_many(struct repository *r, 17 + struct commit *one, size_t n, 18 struct commit **twos, 19 struct commit_list **result); 20 /* To be used only when object flags after this call no longer matter */ 21 int repo_get_merge_bases_many_dirty(struct repository *r, 22 + struct commit *one, size_t n, 23 struct commit **twos, 24 struct commit_list **result); 25
+3 -3
t/helper/test-reach.c
··· 35 struct commit_list *X, *Y; 36 struct object_array X_obj = OBJECT_ARRAY_INIT; 37 struct commit **X_array, **Y_array; 38 - int X_nr, X_alloc, Y_nr, Y_alloc; 39 struct strbuf buf = STRBUF_INIT; 40 struct repository *r = the_repository; 41 ··· 157 clear_contains_cache(&cache); 158 } else if (!strcmp(av[1], "get_reachable_subset")) { 159 const int reachable_flag = 1; 160 - int i, count = 0; 161 struct commit_list *current; 162 struct commit_list *list = get_reachable_subset(X_array, X_nr, 163 Y_array, Y_nr, ··· 169 oid_to_hex(&list->item->object.oid)); 170 count++; 171 } 172 - for (i = 0; i < Y_nr; i++) { 173 if (Y_array[i]->object.flags & reachable_flag) 174 count--; 175 }
··· 35 struct commit_list *X, *Y; 36 struct object_array X_obj = OBJECT_ARRAY_INIT; 37 struct commit **X_array, **Y_array; 38 + size_t X_nr, X_alloc, Y_nr, Y_alloc; 39 struct strbuf buf = STRBUF_INIT; 40 struct repository *r = the_repository; 41 ··· 157 clear_contains_cache(&cache); 158 } else if (!strcmp(av[1], "get_reachable_subset")) { 159 const int reachable_flag = 1; 160 + int count = 0; 161 struct commit_list *current; 162 struct commit_list *list = get_reachable_subset(X_array, X_nr, 163 Y_array, Y_nr, ··· 169 oid_to_hex(&list->item->object.oid)); 170 count++; 171 } 172 + for (size_t i = 0; i < Y_nr; i++) { 173 if (Y_array[i]->object.flags & reachable_flag) 174 count--; 175 }