From 44945dfe867e56aab1685a0f371665273291a2af Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 27 Dec 2024 11:46:21 +0100 Subject: prio-queue: fix type of `insertion_ctr` In 62e745ced2 (prio-queue: use size_t rather than int for size, 2024-12-20), we have converted `struct prio_queue` to use `size_t` to track the number of entries in the queue as well as the allocated size of the underlying array. There is one more counter though, namely the insertion counter, that is still using an `unsigned` instead of a `size_t`. This is unlikely to ever be a problem, but it makes one wonder why some indices use `size_t` while others use `unsigned`. Furthermore, the mentioned commit stated the intent to also adapt these variables, but seemingly forgot to do so. Fix the issue by converting those counters to use `size_t`, as well. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- prio-queue.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/prio-queue.h b/prio-queue.h index 36f370625f..38d032636d 100644 --- a/prio-queue.h +++ b/prio-queue.h @@ -22,13 +22,13 @@ typedef int (*prio_queue_compare_fn)(const void *one, const void *two, void *cb_data); struct prio_queue_entry { - unsigned ctr; + size_t ctr; void *data; }; struct prio_queue { prio_queue_compare_fn compare; - unsigned insertion_ctr; + size_t insertion_ctr; void *cb_data; size_t alloc, nr; struct prio_queue_entry *array; -- cgit v1.2.3 From 95c09e4d07492fa9e4ad951a268b4ea6bae69038 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 27 Dec 2024 11:46:22 +0100 Subject: commit-reach: fix index used to loop through unsigned integer In 62e745ced2 (prio-queue: use size_t rather than int for size, 2024-12-20), we refactored `struct prio_queue` to track the number of contained entries via a `size_t`. While the refactoring adapted one of the users of that variable, it forgot to also adapt "commit-reach.c" accordingly. This was missed because that file has -Wsign-conversion disabled. Fix the issue by using a `size_t` to iterate through entries. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- commit-reach.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/commit-reach.c b/commit-reach.c index e3edd11995..e658726170 100644 --- a/commit-reach.c +++ b/commit-reach.c @@ -42,8 +42,7 @@ static int compare_commits_by_gen(const void *_a, const void *_b) static int queue_has_nonstale(struct prio_queue *queue) { - int i; - for (i = 0; i < queue->nr; i++) { + for (size_t i = 0; i < queue->nr; i++) { struct commit *commit = queue->array[i].data; if (!(commit->object.flags & STALE)) return 1; -- cgit v1.2.3 From 04aeeeaab1f02213703c4e1997b2c2f1ca0f8f96 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 27 Dec 2024 11:46:23 +0100 Subject: commit-reach: fix type of `min_commit_date` The `can_all_from_reach_with_flag()` function accepts a parameter that allows callers to cut off traversal at a specified commit date. This parameter is of type `time_t`, which is a signed type, while we end up comparing it to a commit's `date` field, which is of the unsigned type `timestamp_t`. Fix the parameter to be of type `timestamp_t`. There is only a single caller in "upload-pack.c" that sets this parameter, and that caller knows to pass in a `timestamp_t` already. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- commit-reach.c | 4 ++-- commit-reach.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/commit-reach.c b/commit-reach.c index e658726170..9f8b2457bc 100644 --- a/commit-reach.c +++ b/commit-reach.c @@ -780,7 +780,7 @@ int commit_contains(struct ref_filter *filter, struct commit *commit, int can_all_from_reach_with_flag(struct object_array *from, unsigned int with_flag, unsigned int assign_flag, - time_t min_commit_date, + timestamp_t min_commit_date, timestamp_t min_generation) { struct commit **list = NULL; @@ -883,9 +883,9 @@ int can_all_from_reach(struct commit_list *from, struct commit_list *to, int cutoff_by_min_date) { struct object_array from_objs = OBJECT_ARRAY_INIT; - time_t min_commit_date = cutoff_by_min_date ? from->item->date : 0; struct commit_list *from_iter = from, *to_iter = to; int result; + timestamp_t min_commit_date = cutoff_by_min_date ? from->item->date : 0; timestamp_t min_generation = GENERATION_NUMBER_INFINITY; while (from_iter) { diff --git a/commit-reach.h b/commit-reach.h index 9a745b7e17..d5f3347376 100644 --- a/commit-reach.h +++ b/commit-reach.h @@ -81,7 +81,7 @@ int commit_contains(struct ref_filter *filter, struct commit *commit, int can_all_from_reach_with_flag(struct object_array *from, unsigned int with_flag, unsigned int assign_flag, - time_t min_commit_date, + timestamp_t min_commit_date, timestamp_t min_generation); int can_all_from_reach(struct commit_list *from, struct commit_list *to, int commit_date_cutoff); -- cgit v1.2.3 From 45843d8f4eb2bbfc73cc361ba9d612d088dc8a4f Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 27 Dec 2024 11:46:24 +0100 Subject: commit-reach: use `size_t` to track indices in `remove_redundant()` The function `remove_redundant()` gets as input an array of commits as well as the size of that array and then drops redundant commits from that array. It then returns either `-1` in case an error occurred, or the new number of items in the array. The function receives and returns these sizes with a signed integer, which causes several warnings with -Wsign-compare. Fix this issue by consistently using `size_t` to track array indices and splitting up the returned value into a returned error code and a separate out pointer for the new computed size. Note that `get_merge_bases_many()` and related functions still track array sizes as a signed integer. This will be fixed in a subsequent commit. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- commit-reach.c | 53 ++++++++++++++++++++++++++++++----------------------- 1 file changed, 30 insertions(+), 23 deletions(-) diff --git a/commit-reach.c b/commit-reach.c index 9f8b2457bc..d7f6f1be75 100644 --- a/commit-reach.c +++ b/commit-reach.c @@ -212,12 +212,13 @@ int get_octopus_merge_bases(struct commit_list *in, struct commit_list **result) } static int remove_redundant_no_gen(struct repository *r, - struct commit **array, int cnt) + struct commit **array, + size_t cnt, size_t *dedup_cnt) { struct commit **work; unsigned char *redundant; - int *filled_index; - int i, j, filled; + size_t *filled_index; + size_t i, j, filled; CALLOC_ARRAY(work, cnt); redundant = xcalloc(cnt, 1); @@ -267,20 +268,22 @@ static int remove_redundant_no_gen(struct repository *r, for (i = filled = 0; i < cnt; i++) if (!redundant[i]) array[filled++] = work[i]; + *dedup_cnt = filled; free(work); free(redundant); free(filled_index); - return filled; + return 0; } static int remove_redundant_with_gen(struct repository *r, - struct commit **array, int cnt) + struct commit **array, size_t cnt, + size_t *dedup_cnt) { - int i, count_non_stale = 0, count_still_independent = cnt; + size_t i, count_non_stale = 0, count_still_independent = cnt; timestamp_t min_generation = GENERATION_NUMBER_INFINITY; struct commit **walk_start, **sorted; size_t walk_start_nr = 0, walk_start_alloc = cnt; - int min_gen_pos = 0; + size_t min_gen_pos = 0; /* * Sort the input by generation number, ascending. This allows @@ -326,12 +329,12 @@ static int remove_redundant_with_gen(struct repository *r, * terminate early. Otherwise, we will do the same amount of work * as before. */ - for (i = walk_start_nr - 1; i >= 0 && count_still_independent > 1; i--) { + for (i = walk_start_nr; i && count_still_independent > 1; i--) { /* push the STALE bits up to min generation */ struct commit_list *stack = NULL; - commit_list_insert(walk_start[i], &stack); - walk_start[i]->object.flags |= STALE; + commit_list_insert(walk_start[i - 1], &stack); + walk_start[i - 1]->object.flags |= STALE; while (stack) { struct commit_list *parents; @@ -388,10 +391,12 @@ static int remove_redundant_with_gen(struct repository *r, clear_commit_marks_many(walk_start_nr, walk_start, STALE); free(walk_start); - return count_non_stale; + *dedup_cnt = count_non_stale; + return 0; } -static int remove_redundant(struct repository *r, struct commit **array, int cnt) +static int remove_redundant(struct repository *r, struct commit **array, + size_t cnt, size_t *dedup_cnt) { /* * Some commit in the array may be an ancestor of @@ -401,19 +406,17 @@ static int remove_redundant(struct repository *r, struct commit **array, int cnt * that number. */ if (generation_numbers_enabled(r)) { - int i; - /* * If we have a single commit with finite generation * number, then the _with_gen algorithm is preferred. */ - for (i = 0; i < cnt; i++) { + for (size_t i = 0; i < cnt; i++) { if (commit_graph_generation(array[i]) < GENERATION_NUMBER_INFINITY) - return remove_redundant_with_gen(r, array, cnt); + return remove_redundant_with_gen(r, array, cnt, dedup_cnt); } } - return remove_redundant_no_gen(r, array, cnt); + return remove_redundant_no_gen(r, array, cnt, dedup_cnt); } static int get_merge_bases_many_0(struct repository *r, @@ -425,7 +428,8 @@ static int get_merge_bases_many_0(struct repository *r, { struct commit_list *list; struct commit **rslt; - int cnt, i; + size_t cnt, i; + int ret; if (merge_bases_many(r, one, n, twos, result) < 0) return -1; @@ -452,8 +456,8 @@ static int get_merge_bases_many_0(struct repository *r, clear_commit_marks(one, all_flags); clear_commit_marks_many(n, twos, all_flags); - cnt = remove_redundant(r, rslt, cnt); - if (cnt < 0) { + ret = remove_redundant(r, rslt, cnt, &cnt); + if (ret < 0) { free(rslt); return -1; } @@ -582,7 +586,8 @@ struct commit_list *reduce_heads(struct commit_list *heads) struct commit_list *p; struct commit_list *result = NULL, **tail = &result; struct commit **array; - int num_head, i; + size_t num_head, i; + int ret; if (!heads) return NULL; @@ -603,11 +608,13 @@ struct commit_list *reduce_heads(struct commit_list *heads) p->item->object.flags &= ~STALE; } } - num_head = remove_redundant(the_repository, array, num_head); - if (num_head < 0) { + + ret = remove_redundant(the_repository, array, num_head, &num_head); + if (ret < 0) { free(array); return NULL; } + for (i = 0; i < num_head; i++) tail = &commit_list_insert(array[i], tail)->next; free(array); -- cgit v1.2.3 From 85ee0680e2d5d667919e06394ca7622f09652310 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 27 Dec 2024 11:46:25 +0100 Subject: commit-reach: use `size_t` to track indices in `get_reachable_subset()` Similar as with the preceding commit, adapt `get_reachable_subset()` so that it tracks array indices via `size_t` instead of using signed integers to fix a couple of -Wsign-compare warnings. Adapt callers accordingly. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- bisect.c | 9 +++++---- commit-reach.c | 8 ++++---- commit-reach.h | 4 ++-- commit.c | 4 ++-- commit.h | 2 +- ref-filter.c | 2 +- remote.c | 4 ++-- 7 files changed, 17 insertions(+), 16 deletions(-) diff --git a/bisect.c b/bisect.c index 1a9069c9ad..7a1afc46e5 100644 --- a/bisect.c +++ b/bisect.c @@ -780,10 +780,10 @@ static struct commit *get_commit_reference(struct repository *r, } static struct commit **get_bad_and_good_commits(struct repository *r, - int *rev_nr) + size_t *rev_nr) { struct commit **rev; - int i, n = 0; + size_t i, n = 0; ALLOC_ARRAY(rev, 1 + good_revs.nr); rev[n++] = get_commit_reference(r, current_bad_oid); @@ -887,7 +887,7 @@ static enum bisect_error check_merge_bases(int rev_nr, struct commit **rev, int return res; } -static int check_ancestors(struct repository *r, int rev_nr, +static int check_ancestors(struct repository *r, size_t rev_nr, struct commit **rev, const char *prefix) { struct strvec rev_argv = STRVEC_INIT; @@ -922,7 +922,8 @@ static enum bisect_error check_good_are_ancestors_of_bad(struct repository *r, { char *filename; struct stat st; - int fd, rev_nr; + int fd; + size_t rev_nr; enum bisect_error res = BISECT_OK; struct commit **rev; diff --git a/commit-reach.c b/commit-reach.c index d7f6f1be75..bab40f5575 100644 --- a/commit-reach.c +++ b/commit-reach.c @@ -791,8 +791,8 @@ int can_all_from_reach_with_flag(struct object_array *from, timestamp_t min_generation) { struct commit **list = NULL; - int i; - int nr_commits; + size_t i; + size_t nr_commits; int result = 1; ALLOC_ARRAY(list, from->nr); @@ -944,8 +944,8 @@ int can_all_from_reach(struct commit_list *from, struct commit_list *to, return result; } -struct commit_list *get_reachable_subset(struct commit **from, int nr_from, - struct commit **to, int nr_to, +struct commit_list *get_reachable_subset(struct commit **from, size_t nr_from, + struct commit **to, size_t nr_to, unsigned int reachable_flag) { struct commit **item; diff --git a/commit-reach.h b/commit-reach.h index d5f3347376..fa5408054a 100644 --- a/commit-reach.h +++ b/commit-reach.h @@ -95,8 +95,8 @@ int can_all_from_reach(struct commit_list *from, struct commit_list *to, * This method uses the PARENT1 and PARENT2 flags during its operation, * so be sure these flags are not set before calling the method. */ -struct commit_list *get_reachable_subset(struct commit **from, int nr_from, - struct commit **to, int nr_to, +struct commit_list *get_reachable_subset(struct commit **from, size_t nr_from, + struct commit **to, size_t nr_to, unsigned int reachable_flag); struct ahead_behind_count { diff --git a/commit.c b/commit.c index a127fe60c5..540660359d 100644 --- a/commit.c +++ b/commit.c @@ -778,11 +778,11 @@ static void clear_commit_marks_1(struct commit_list **plist, } } -void clear_commit_marks_many(int nr, struct commit **commit, unsigned int mark) +void clear_commit_marks_many(size_t nr, struct commit **commit, unsigned int mark) { struct commit_list *list = NULL; - while (nr--) { + for (size_t i = 0; i < nr; i++) { clear_commit_marks_1(&list, *commit, mark); commit++; } diff --git a/commit.h b/commit.h index 943e3d74b2..70c870dae4 100644 --- a/commit.h +++ b/commit.h @@ -210,7 +210,7 @@ struct commit *pop_most_recent_commit(struct commit_list **list, struct commit *pop_commit(struct commit_list **stack); void clear_commit_marks(struct commit *commit, unsigned int mark); -void clear_commit_marks_many(int nr, struct commit **commit, unsigned int mark); +void clear_commit_marks_many(size_t nr, struct commit **commit, unsigned int mark); enum rev_sort_order { diff --git a/ref-filter.c b/ref-filter.c index 23054694c2..bf5534605e 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -3041,7 +3041,7 @@ static void reach_filter(struct ref_array *array, struct commit_list **check_reachable, int include_reached) { - int i, old_nr; + size_t i, old_nr; struct commit **to_clear; if (!*check_reachable) diff --git a/remote.c b/remote.c index 18e5ccf391..0f6fba8562 100644 --- a/remote.c +++ b/remote.c @@ -1535,7 +1535,7 @@ static struct ref **tail_ref(struct ref **head) struct tips { struct commit **tip; - int nr, alloc; + size_t nr, alloc; }; static void add_to_tips(struct tips *tips, const struct object_id *oid) @@ -1602,7 +1602,7 @@ static void add_missing_tags(struct ref *src, struct ref **dst, struct ref ***ds const int reachable_flag = 1; struct commit_list *found_commits; struct commit **src_commits; - int nr_src_commits = 0, alloc_src_commits = 16; + size_t nr_src_commits = 0, alloc_src_commits = 16; ALLOC_ARRAY(src_commits, alloc_src_commits); for_each_string_list_item(item, &src_tag) { -- cgit v1.2.3 From 0905ed201a87bc97dc4d47c0cb8fd65316f33269 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 27 Dec 2024 11:46:26 +0100 Subject: builtin/log: use `size_t` to track indices Similar as with the preceding commit, adapt "builtin/log.c" so that it tracks array indices via `size_t` instead of using signed integers. This fixes a couple of -Wsign-compare warnings and prepares the code for a similar refactoring of `repo_get_merge_bases_many()` in a subsequent commit. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/log.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/builtin/log.c b/builtin/log.c index 75e1b34123..805b2355d9 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1746,11 +1746,12 @@ struct base_tree_info { static struct commit *get_base_commit(const struct format_config *cfg, struct commit **list, - int total) + size_t total) { struct commit *base = NULL; struct commit **rev; - int i = 0, rev_nr = 0, auto_select, die_on_failure, ret; + int auto_select, die_on_failure, ret; + size_t i = 0, rev_nr = 0; switch (cfg->auto_base) { case AUTO_BASE_NEVER: @@ -1885,13 +1886,12 @@ define_commit_slab(commit_base, int); static void prepare_bases(struct base_tree_info *bases, struct commit *base, struct commit **list, - int total) + size_t total) { struct commit *commit; struct rev_info revs; struct diff_options diffopt; struct commit_base commit_base; - int i; if (!base) return; @@ -1906,7 +1906,7 @@ static void prepare_bases(struct base_tree_info *bases, repo_init_revisions(the_repository, &revs, NULL); revs.max_parents = 1; revs.topo_order = 1; - for (i = 0; i < total; i++) { + for (size_t i = 0; i < total; i++) { list[i]->object.flags &= ~UNINTERESTING; add_pending_object(&revs, &list[i]->object, "rev_list"); *commit_base_at(&commit_base, list[i]) = 1; @@ -2007,7 +2007,7 @@ int cmd_format_patch(int argc, struct rev_info rev; char *to_free = NULL; struct setup_revision_opt s_r_opt; - int nr = 0, total, i; + size_t nr = 0, total, i; int use_stdout = 0; int start_number = -1; int just_numbers = 0; @@ -2500,11 +2500,14 @@ int cmd_format_patch(int argc, if (show_progress) progress = start_delayed_progress(_("Generating patches"), total); - while (0 <= --nr) { + for (i = 0; i < nr; i++) { + size_t idx = nr - i - 1; int shown; - display_progress(progress, total - nr); - commit = list[nr]; - rev.nr = total - nr + (start_number - 1); + + display_progress(progress, total - idx); + commit = list[idx]; + rev.nr = total - idx + (start_number - 1); + /* Make the second and subsequent mails replies to the first */ if (cfg.thread) { /* Have we already had a message ID? */ -- cgit v1.2.3 From 1ab5948141e62b52bcb812b04a901b3efaf1b578 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 27 Dec 2024 11:46:27 +0100 Subject: builtin/log: fix remaining -Wsign-compare warnings Fix remaining -Wsign-compare warnings in "builtin/log.c" and mark the file as -Wsign-compare-clean. While most of the fixes are obvious, one fix requires us to use `cast_size_t_to_int()`, which will cause us to die in case the `size_t` cannot be represented as `int`. This should be fine though, as the data would typically be set either via a config key or via the command line, neither of which should ever exceed a couple of kilobytes of data. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/log.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/builtin/log.c b/builtin/log.c index 805b2355d9..a4f41aafca 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -6,7 +6,6 @@ */ #define USE_THE_REPOSITORY_VARIABLE -#define DISABLE_SIGN_COMPARE_WARNINGS #include "builtin.h" #include "abspath.h" @@ -209,7 +208,6 @@ static void cmd_log_init_defaults(struct rev_info *rev, static void set_default_decoration_filter(struct decoration_filter *decoration_filter) { - int i; char *value = NULL; struct string_list *include = decoration_filter->include_ref_pattern; const struct string_list *config_exclude; @@ -243,7 +241,7 @@ static void set_default_decoration_filter(struct decoration_filter *decoration_f * No command-line or config options were given, so * populate with sensible defaults. */ - for (i = 0; i < ARRAY_SIZE(ref_namespace); i++) { + for (size_t i = 0; i < ARRAY_SIZE(ref_namespace); i++) { if (!ref_namespace[i].decoration) continue; @@ -717,14 +715,14 @@ static int show_tag_object(const struct object_id *oid, struct rev_info *rev) unsigned long size; enum object_type type; char *buf = repo_read_object_file(the_repository, oid, &type, &size); - int offset = 0; + unsigned long offset = 0; if (!buf) return error(_("could not read object %s"), oid_to_hex(oid)); assert(type == OBJ_TAG); while (offset < size && buf[offset] != '\n') { - int new_offset = offset + 1; + unsigned long new_offset = offset + 1; const char *ident; while (new_offset < size && buf[new_offset++] != '\n') ; /* do nothing */ @@ -1316,24 +1314,25 @@ static void print_signature(const char *signature, FILE *file) static char *find_branch_name(struct rev_info *rev) { - int i, positive = -1; struct object_id branch_oid; const struct object_id *tip_oid; const char *ref, *v; char *full_ref, *branch = NULL; + int interesting_found = 0; + size_t idx; - for (i = 0; i < rev->cmdline.nr; i++) { + for (size_t i = 0; i < rev->cmdline.nr; i++) { if (rev->cmdline.rev[i].flags & UNINTERESTING) continue; - if (positive < 0) - positive = i; - else + if (interesting_found) return NULL; + interesting_found = 1; + idx = i; } - if (positive < 0) + if (!interesting_found) return NULL; - ref = rev->cmdline.rev[positive].name; - tip_oid = &rev->cmdline.rev[positive].item->oid; + ref = rev->cmdline.rev[idx].name; + tip_oid = &rev->cmdline.rev[idx].item->oid; if (repo_dwim_ref(the_repository, ref, strlen(ref), &branch_oid, &full_ref, 0) && skip_prefix(full_ref, "refs/heads/", &v) && @@ -2183,7 +2182,7 @@ int cmd_format_patch(int argc, fmt_patch_suffix = cfg.fmt_patch_suffix; /* Make sure "0000-$sub.patch" gives non-negative length for $sub */ - if (cfg.log.fmt_patch_name_max <= strlen("0000-") + strlen(fmt_patch_suffix)) + if (cfg.log.fmt_patch_name_max <= cast_size_t_to_int(strlen("0000-") + strlen(fmt_patch_suffix))) cfg.log.fmt_patch_name_max = strlen("0000-") + strlen(fmt_patch_suffix); if (cover_from_description_arg) -- cgit v1.2.3 From 455ac07021d4feede4f5b7e39bf00dc186ce3c09 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 27 Dec 2024 11:46:28 +0100 Subject: shallow: fix -Wsign-compare warnings Fix a couple of -Wsign-compare issues in "shallow.c" and mark the file as -Wsign-compare-clean. This change prepares the code for a refactoring of `repo_in_merge_bases_many()`, which will be adapted to accept the number of commits as `size_t` instead of `int`. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- shallow.c | 38 ++++++++++++++++++-------------------- shallow.h | 6 +++--- 2 files changed, 21 insertions(+), 23 deletions(-) diff --git a/shallow.c b/shallow.c index 82a8da3d73..b8fcfbef0f 100644 --- a/shallow.c +++ b/shallow.c @@ -1,5 +1,4 @@ #define USE_THE_REPOSITORY_VARIABLE -#define DISABLE_SIGN_COMPARE_WARNINGS #include "git-compat-util.h" #include "hex.h" @@ -134,7 +133,8 @@ static void free_depth_in_slab(int **ptr) struct commit_list *get_shallow_commits(struct object_array *heads, int depth, int shallow_flag, int not_shallow_flag) { - int i = 0, cur_depth = 0; + size_t i = 0; + int cur_depth = 0; struct commit_list *result = NULL; struct object_array stack = OBJECT_ARRAY_INIT; struct commit *commit = NULL; @@ -335,16 +335,16 @@ static int write_shallow_commits_1(struct strbuf *out, int use_pack_protocol, const struct oid_array *extra, unsigned flags) { - struct write_shallow_data data; - int i; - data.out = out; - data.use_pack_protocol = use_pack_protocol; - data.count = 0; - data.flags = flags; + struct write_shallow_data data = { + .out = out, + .use_pack_protocol = use_pack_protocol, + .flags = flags, + }; + for_each_commit_graft(write_one_shallow, &data); if (!extra) return data.count; - for (i = 0; i < extra->nr; i++) { + for (size_t i = 0; i < extra->nr; i++) { strbuf_addstr(out, oid_to_hex(extra->oid + i)); strbuf_addch(out, '\n'); data.count++; @@ -466,7 +466,6 @@ struct trace_key trace_shallow = TRACE_KEY_INIT(SHALLOW); */ void prepare_shallow_info(struct shallow_info *info, struct oid_array *sa) { - int i; trace_printf_key(&trace_shallow, "shallow: prepare_shallow_info\n"); memset(info, 0, sizeof(*info)); info->shallow = sa; @@ -474,7 +473,7 @@ void prepare_shallow_info(struct shallow_info *info, struct oid_array *sa) return; ALLOC_ARRAY(info->ours, sa->nr); ALLOC_ARRAY(info->theirs, sa->nr); - for (i = 0; i < sa->nr; i++) { + for (size_t i = 0; i < sa->nr; i++) { if (repo_has_object_file(the_repository, sa->oid + i)) { struct commit_graft *graft; graft = lookup_commit_graft(the_repository, @@ -507,7 +506,7 @@ void clear_shallow_info(struct shallow_info *info) void remove_nonexistent_theirs_shallow(struct shallow_info *info) { struct object_id *oid = info->shallow->oid; - int i, dst; + size_t i, dst; trace_printf_key(&trace_shallow, "shallow: remove_nonexistent_theirs_shallow\n"); for (i = dst = 0; i < info->nr_theirs; i++) { if (i != dst) @@ -560,7 +559,7 @@ static void paint_down(struct paint_info *info, const struct object_id *oid, { unsigned int i, nr; struct commit_list *head = NULL; - int bitmap_nr = DIV_ROUND_UP(info->nr_bits, 32); + size_t bitmap_nr = DIV_ROUND_UP(info->nr_bits, 32); size_t bitmap_size = st_mult(sizeof(uint32_t), bitmap_nr); struct commit *c = lookup_commit_reference_gently(the_repository, oid, 1); @@ -660,7 +659,7 @@ void assign_shallow_commits_to_refs(struct shallow_info *info, struct object_id *oid = info->shallow->oid; struct oid_array *ref = info->ref; unsigned int i, nr; - int *shallow, nr_shallow = 0; + size_t *shallow, nr_shallow = 0; struct paint_info pi; trace_printf_key(&trace_shallow, "shallow: assign_shallow_commits_to_refs\n"); @@ -735,7 +734,7 @@ void assign_shallow_commits_to_refs(struct shallow_info *info, struct commit_array { struct commit **commits; - int nr, alloc; + size_t nr, alloc; }; static int add_ref(const char *refname UNUSED, @@ -753,12 +752,11 @@ static int add_ref(const char *refname UNUSED, return 0; } -static void update_refstatus(int *ref_status, int nr, uint32_t *bitmap) +static void update_refstatus(int *ref_status, size_t nr, uint32_t *bitmap) { - unsigned int i; if (!ref_status) return; - for (i = 0; i < nr; i++) + for (size_t i = 0; i < nr; i++) if (bitmap[i / 32] & (1U << (i % 32))) ref_status[i]++; } @@ -773,8 +771,8 @@ static void post_assign_shallow(struct shallow_info *info, struct object_id *oid = info->shallow->oid; struct commit *c; uint32_t **bitmap; - int dst, i, j; - int bitmap_nr = DIV_ROUND_UP(info->ref->nr, 32); + size_t dst, i, j; + size_t bitmap_nr = DIV_ROUND_UP(info->ref->nr, 32); struct commit_array ca; trace_printf_key(&trace_shallow, "shallow: post_assign_shallow\n"); diff --git a/shallow.h b/shallow.h index e9ca7e4bc8..9bfeade93e 100644 --- a/shallow.h +++ b/shallow.h @@ -59,8 +59,8 @@ void prune_shallow(unsigned options); */ struct shallow_info { struct oid_array *shallow; - int *ours, nr_ours; - int *theirs, nr_theirs; + size_t *ours, nr_ours; + size_t *theirs, nr_theirs; struct oid_array *ref; /* for receive-pack */ @@ -69,7 +69,7 @@ struct shallow_info { int *reachable; int *shallow_ref; struct commit **commits; - int nr_commits; + size_t nr_commits; }; void prepare_shallow_info(struct shallow_info *, struct oid_array *); -- cgit v1.2.3 From 5e7fe8a7b89a07d8c3ab298ac69bc33f6ba88b47 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 27 Dec 2024 11:46:29 +0100 Subject: 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 Signed-off-by: Junio C Hamano --- bisect.c | 2 +- builtin/merge-base.c | 4 ++-- commit-reach.c | 7 +++---- commit-reach.h | 4 ++-- t/helper/test-reach.c | 6 +++--- 5 files changed, 11 insertions(+), 12 deletions(-) diff --git a/bisect.c b/bisect.c index 7a1afc46e5..7a3c77c6d8 100644 --- 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 a20c93b11a..123c81515e 100644 --- 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 bab40f5575..a339e41aa4 100644 --- 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 fa5408054a..6012402dfc 100644 --- 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 01cf77ae65..028ec00306 100644 --- 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--; } -- cgit v1.2.3 From 24027256aa9614a445563707a72af7ce5ff49b5b Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 27 Dec 2024 12:25:30 -0800 Subject: sign-compare: avoid comparing ptrdiff with an int/unsigned Instead, offset the base pointer with integer and compare it with the other pointer. Signed-off-by: Junio C Hamano --- shallow.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shallow.c b/shallow.c index b8fcfbef0f..b54244ffa9 100644 --- a/shallow.c +++ b/shallow.c @@ -534,7 +534,7 @@ static uint32_t *paint_alloc(struct paint_info *info) unsigned nr = DIV_ROUND_UP(info->nr_bits, 32); unsigned size = nr * sizeof(uint32_t); void *p; - if (!info->pool_count || size > info->end - info->free) { + if (!info->pool_count || info->end < info->free + size) { if (size > POOL_SIZE) BUG("pool size too small for %d in paint_alloc()", size); -- cgit v1.2.3