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>

diff --git a/bisect.c b/bisect.c
index 7a1afc46e5fc0250212f5b6eaf952cf8e36b56fe..7a3c77c6d84da0cb6e135b6e0b1ca3596903af5c 100644 (file)
--- a/bisect.c
+++ b/bisect.c
@@ -855,7 +855,7 @@ static void handle_skipped_merge_base(const struct object_id *mb)
  * for early success, this will be converted back to 0 in
  * check_good_are_ancestors_of_bad().
  */
-static enum bisect_error check_merge_bases(int rev_nr, struct commit **rev, int no_checkout)
+static enum bisect_error check_merge_bases(size_t rev_nr, struct commit **rev, int no_checkout)
 {
        enum bisect_error res = BISECT_OK;
        struct commit_list *result = NULL;

diff --git a/builtin/merge-base.c b/builtin/merge-base.c
index a20c93b11aaa0b9f380b843469da2a3b5db10d00..123c81515e1f5f12045e4073f78b6e8a2b04bb4b 100644 (file)
--- a/builtin/merge-base.c
+++ b/builtin/merge-base.c
@@ -8,7 +8,7 @@
 #include "parse-options.h"
 #include "commit-reach.h"
 
-static int show_merge_base(struct commit **rev, int rev_nr, int show_all)
+static int show_merge_base(struct commit **rev, size_t rev_nr, int show_all)
 {
        struct commit_list *result = NULL, *r;
 
@@ -149,7 +149,7 @@ int cmd_merge_base(int argc,
                   struct repository *repo UNUSED)
 {
        struct commit **rev;
-       int rev_nr = 0;
+       size_t rev_nr = 0;
        int show_all = 0;
        int cmdmode = 0;
        int ret;

diff --git a/commit-reach.c b/commit-reach.c
index bab40f557580476d59d3a0b0ef56f40263e6615e..a339e41aa4ed1e375ee6f2f42a163ff1c654c3e4 100644 (file)
--- a/commit-reach.c
+++ b/commit-reach.c
@@ -1,5 +1,4 @@
 #define USE_THE_REPOSITORY_VARIABLE
-#define DISABLE_SIGN_COMPARE_WARNINGS
 
 #include "git-compat-util.h"
 #include "commit.h"
@@ -421,7 +420,7 @@ static int remove_redundant(struct repository *r, struct commit **array,
 
 static int get_merge_bases_many_0(struct repository *r,
                                  struct commit *one,
-                                 int n,
+                                 size_t n,
                                  struct commit **twos,
                                  int cleanup,
                                  struct commit_list **result)
@@ -469,7 +468,7 @@ static int get_merge_bases_many_0(struct repository *r,
 
 int repo_get_merge_bases_many(struct repository *r,
                              struct commit *one,
-                             int n,
+                             size_t n,
                              struct commit **twos,
                              struct commit_list **result)
 {
@@ -478,7 +477,7 @@ int repo_get_merge_bases_many(struct repository *r,
 
 int repo_get_merge_bases_many_dirty(struct repository *r,
                                    struct commit *one,
-                                   int n,
+                                   size_t n,
                                    struct commit **twos,
                                    struct commit_list **result)
 {

diff --git a/commit-reach.h b/commit-reach.h
index fa5408054ac01372c041d18595a405cfdaec6af3..6012402dfcfe453fd710d0b4c9a9e09f8953f63a 100644 (file)
--- a/commit-reach.h
+++ b/commit-reach.h
@@ -14,12 +14,12 @@ int repo_get_merge_bases(struct repository *r,
                         struct commit *rev2,
                         struct commit_list **result);
 int repo_get_merge_bases_many(struct repository *r,
-                             struct commit *one, int n,
+                             struct commit *one, size_t n,
                              struct commit **twos,
                              struct commit_list **result);
 /* To be used only when object flags after this call no longer matter */
 int repo_get_merge_bases_many_dirty(struct repository *r,
-                                   struct commit *one, int n,
+                                   struct commit *one, size_t n,
                                    struct commit **twos,
                                    struct commit_list **result);
 

diff --git a/t/helper/test-reach.c b/t/helper/test-reach.c
index 01cf77ae65b9a7db2f31a9a1558b8bb84b2e81d3..028ec0030678284eba844e121c6eff88abdd3139 100644 (file)
--- a/t/helper/test-reach.c
+++ b/t/helper/test-reach.c
@@ -35,7 +35,7 @@ int cmd__reach(int ac, const char **av)
        struct commit_list *X, *Y;
        struct object_array X_obj = OBJECT_ARRAY_INIT;
        struct commit **X_array, **Y_array;
-       int X_nr, X_alloc, Y_nr, Y_alloc;
+       size_t X_nr, X_alloc, Y_nr, Y_alloc;
        struct strbuf buf = STRBUF_INIT;
        struct repository *r = the_repository;
 
@@ -157,7 +157,7 @@ int cmd__reach(int ac, const char **av)
                clear_contains_cache(&cache);
        } else if (!strcmp(av[1], "get_reachable_subset")) {
                const int reachable_flag = 1;
-               int i, count = 0;
+               int count = 0;
                struct commit_list *current;
                struct commit_list *list = get_reachable_subset(X_array, X_nr,
                                                                Y_array, Y_nr,
@@ -169,7 +169,7 @@ int cmd__reach(int ac, const char **av)
                                    oid_to_hex(&list->item->object.oid));
                        count++;
                }
-               for (i = 0; i < Y_nr; i++) {
+               for (size_t i = 0; i < Y_nr; i++) {
                        if (Y_array[i]->object.flags & reachable_flag)
                                count--;
                }