git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: git@vger.kernel.org
Subject: [PATCH 4/9] commit-graph: fix sign comparison warnings
Date: Mon, 04 Aug 2025 10:17:20 +0200	[thread overview]
Message-ID: <20250804-b4-pks-commit-graph-wo-the-repository-v1-4-850d626eb2e8@pks.im> (raw)
In-Reply-To: <20250804-b4-pks-commit-graph-wo-the-repository-v1-0-850d626eb2e8@pks.im>

The "commit-graph.c" file has a bunch of sign comparison warnings:

  - There are a bunch of variables that are declared as signed integers
    even though they are used to count entities, like for example
    `num_commit_graphs_before` and `num_commit_graphs_after`.

  - There are several cases where we use signed loop variables to
    iterate through an unsigned entity count.

  - In `write_graph_chunk_base_1()` we count how many chunks we have
    written in total. But while the value represents a positive
    quantity, we still return a signed integer that we then later
    compare with unsigned values.

  - The bloom settings hash version is being assigned `-1` even though
    it's an unsigned value. This is used to indicate an unspecified
    value and relies on 1's complement.

Fix all of these cases by either using the proper variable type or by
adding casts as required.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 commit-graph.c | 54 +++++++++++++++++++++++++++---------------------------
 1 file changed, 27 insertions(+), 27 deletions(-)

diff --git a/commit-graph.c b/commit-graph.c
index ad3f084dd4..443177ffd3 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -1,5 +1,4 @@
 #define USE_THE_REPOSITORY_VARIABLE
-#define DISABLE_SIGN_COMPARE_WARNINGS
 
 #include "git-compat-util.h"
 #include "config.h"
@@ -569,7 +568,7 @@ static void validate_mixed_bloom_settings(struct commit_graph *g)
 static int add_graph_to_chain(struct commit_graph *g,
 			      struct commit_graph *chain,
 			      struct object_id *oids,
-			      int n)
+			      size_t n)
 {
 	struct commit_graph *cur_g = chain;
 
@@ -622,7 +621,7 @@ int open_commit_graph_chain(const char *chain_file,
 		close(*fd);
 		return 0;
 	}
-	if (st->st_size < the_hash_algo->hexsz) {
+	if (st->st_size < (ssize_t) the_hash_algo->hexsz) {
 		close(*fd);
 		if (!st->st_size) {
 			/* treat empty files the same as missing */
@@ -643,15 +642,16 @@ struct commit_graph *load_commit_graph_chain_fd_st(struct repository *r,
 	struct commit_graph *graph_chain = NULL;
 	struct strbuf line = STRBUF_INIT;
 	struct object_id *oids;
-	int i = 0, valid = 1, count;
+	int valid = 1;
 	FILE *fp = xfdopen(fd, "r");
+	size_t count;
 
 	count = st->st_size / (the_hash_algo->hexsz + 1);
 	CALLOC_ARRAY(oids, count);
 
 	odb_prepare_alternates(r->objects);
 
-	for (i = 0; i < count; i++) {
+	for (size_t i = 0; i < count; i++) {
 		struct odb_source *source;
 
 		if (strbuf_getline_lf(&line, fp) == EOF)
@@ -1145,12 +1145,12 @@ struct write_commit_graph_context {
 	int num_generation_data_overflows;
 	unsigned long approx_nr_objects;
 	struct progress *progress;
-	int progress_done;
+	uint64_t progress_done;
 	uint64_t progress_cnt;
 
 	char *base_graph_name;
-	int num_commit_graphs_before;
-	int num_commit_graphs_after;
+	size_t num_commit_graphs_before;
+	size_t num_commit_graphs_after;
 	char **commit_graph_filenames_before;
 	char **commit_graph_filenames_after;
 	char **commit_graph_hash_after;
@@ -1181,7 +1181,7 @@ static int write_graph_chunk_fanout(struct hashfile *f,
 				    void *data)
 {
 	struct write_commit_graph_context *ctx = data;
-	int i, count = 0;
+	size_t i, count = 0;
 	struct commit **list = ctx->commits.list;
 
 	/*
@@ -1209,7 +1209,8 @@ static int write_graph_chunk_oids(struct hashfile *f,
 {
 	struct write_commit_graph_context *ctx = data;
 	struct commit **list = ctx->commits.list;
-	int count;
+	size_t count;
+
 	for (count = 0; count < ctx->commits.nr; count++, list++) {
 		display_progress(ctx->progress, ++ctx->progress_cnt);
 		hashwrite(f, (*list)->object.oid.hash, the_hash_algo->rawsz);
@@ -1331,9 +1332,9 @@ static int write_graph_chunk_generation_data(struct hashfile *f,
 					     void *data)
 {
 	struct write_commit_graph_context *ctx = data;
-	int i, num_generation_data_overflows = 0;
+	int num_generation_data_overflows = 0;
 
-	for (i = 0; i < ctx->commits.nr; i++) {
+	for (size_t i = 0; i < ctx->commits.nr; i++) {
 		struct commit *c = ctx->commits.list[i];
 		timestamp_t offset;
 		repo_parse_commit(ctx->r, c);
@@ -1355,8 +1356,8 @@ static int write_graph_chunk_generation_data_overflow(struct hashfile *f,
 						      void *data)
 {
 	struct write_commit_graph_context *ctx = data;
-	int i;
-	for (i = 0; i < ctx->commits.nr; i++) {
+
+	for (size_t i = 0; i < ctx->commits.nr; i++) {
 		struct commit *c = ctx->commits.list[i];
 		timestamp_t offset = commit_graph_data_at(c)->generation - c->date;
 		display_progress(ctx->progress, ++ctx->progress_cnt);
@@ -1526,7 +1527,7 @@ static void add_missing_parents(struct write_commit_graph_context *ctx, struct c
 
 static void close_reachable(struct write_commit_graph_context *ctx)
 {
-	int i;
+	size_t i;
 	struct commit *commit;
 	enum commit_graph_split_flags flags = ctx->opts ?
 		ctx->opts->split_flags : COMMIT_GRAPH_SPLIT_UNSPECIFIED;
@@ -1620,10 +1621,9 @@ static void compute_reachable_generation_numbers(
 			struct compute_generation_info *info,
 			int generation_version)
 {
-	int i;
 	struct commit_list *list = NULL;
 
-	for (i = 0; i < info->commits->nr; i++) {
+	for (size_t i = 0; i < info->commits->nr; i++) {
 		struct commit *c = info->commits->list[i];
 		timestamp_t gen;
 		repo_parse_commit(info->r, c);
@@ -1714,7 +1714,7 @@ static void set_generation_v2(struct commit *c, timestamp_t t,
 
 static void compute_generation_numbers(struct write_commit_graph_context *ctx)
 {
-	int i;
+	size_t i;
 	struct compute_generation_info info = {
 		.r = ctx->r,
 		.commits = &ctx->commits,
@@ -1793,10 +1793,10 @@ static void trace2_bloom_filter_write_statistics(struct write_commit_graph_conte
 
 static void compute_bloom_filters(struct write_commit_graph_context *ctx)
 {
-	int i;
+	size_t i;
 	struct progress *progress = NULL;
 	struct commit **sorted_commits;
-	int max_new_filters;
+	size_t max_new_filters;
 
 	init_bloom_filters();
 
@@ -1814,7 +1814,7 @@ static void compute_bloom_filters(struct write_commit_graph_context *ctx)
 		QSORT(sorted_commits, ctx->commits.nr, commit_gen_cmp);
 
 	max_new_filters = ctx->opts && ctx->opts->max_new_filters >= 0 ?
-		ctx->opts->max_new_filters : ctx->commits.nr;
+		(size_t) ctx->opts->max_new_filters : ctx->commits.nr;
 
 	for (i = 0; i < ctx->commits.nr; i++) {
 		enum bloom_filter_computed computed = 0;
@@ -2017,10 +2017,10 @@ static void copy_oids_to_commits(struct write_commit_graph_context *ctx)
 	stop_progress(&ctx->progress);
 }
 
-static int write_graph_chunk_base_1(struct hashfile *f,
-				    struct commit_graph *g)
+static size_t write_graph_chunk_base_1(struct hashfile *f,
+				       struct commit_graph *g)
 {
-	int num = 0;
+	size_t num = 0;
 
 	if (!g)
 		return 0;
@@ -2034,7 +2034,7 @@ static int write_graph_chunk_base(struct hashfile *f,
 				    void *data)
 {
 	struct write_commit_graph_context *ctx = data;
-	int num = write_graph_chunk_base_1(f, ctx->new_base_graph);
+	size_t num = write_graph_chunk_base_1(f, ctx->new_base_graph);
 
 	if (num != ctx->num_commit_graphs_after - 1) {
 		error(_("failed to write correct number of base graph ids"));
@@ -2480,7 +2480,7 @@ static void expire_commit_graphs(struct write_commit_graph_context *ctx)
 		if (stat(path.buf, &st) < 0)
 			continue;
 
-		if (st.st_mtime > expire_time)
+		if ((unsigned) st.st_mtime > expire_time)
 			continue;
 		if (path.len < 6 || strcmp(path.buf + path.len - 6, ".graph"))
 			continue;
@@ -2576,7 +2576,7 @@ int write_commit_graph(struct odb_source *source,
 			ctx.changed_paths = 1;
 
 			/* don't propagate the hash_version unless unspecified */
-			if (bloom_settings.hash_version == -1)
+			if (bloom_settings.hash_version == (unsigned) -1)
 				bloom_settings.hash_version = g->bloom_filter_settings->hash_version;
 			bloom_settings.bits_per_entry = g->bloom_filter_settings->bits_per_entry;
 			bloom_settings.num_hashes = g->bloom_filter_settings->num_hashes;

-- 
2.50.1.723.g3e08bea96f.dirty


  parent reply	other threads:[~2025-08-04  8:17 UTC|newest]

Thread overview: 69+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-04  8:17 [PATCH 0/9] commit-graph: remove reliance on global state Patrick Steinhardt
2025-08-04  8:17 ` [PATCH 1/9] trace2: introduce function to trace unsigned integers Patrick Steinhardt
2025-08-04 21:33   ` Taylor Blau
2025-08-04  8:17 ` [PATCH 2/9] commit-graph: stop using signed integers to count bloom filters Patrick Steinhardt
2025-08-04  9:13   ` Oswald Buddenhagen
2025-08-04 11:18     ` Patrick Steinhardt
2025-08-04 18:34       ` Junio C Hamano
2025-08-04 21:44         ` Taylor Blau
2025-08-06  6:23           ` Patrick Steinhardt
2025-08-06 12:54             ` Oswald Buddenhagen
2025-08-06 19:04               ` Junio C Hamano
2025-08-06 15:41             ` Junio C Hamano
2025-08-07  7:04               ` Patrick Steinhardt
2025-08-07 22:41                 ` Junio C Hamano
2025-08-11  8:05                   ` Patrick Steinhardt
2025-08-05 15:13         ` Junio C Hamano
2025-08-04 21:42   ` Taylor Blau
2025-08-04  8:17 ` [PATCH 3/9] commit-graph: fix type for some write options Patrick Steinhardt
2025-08-04 21:52   ` Taylor Blau
2025-08-04  8:17 ` Patrick Steinhardt [this message]
2025-08-04 22:04   ` [PATCH 4/9] commit-graph: fix sign comparison warnings Taylor Blau
2025-08-06  6:52     ` Patrick Steinhardt
2025-08-04  8:17 ` [PATCH 5/9] commit-graph: stop using `the_hash_algo` via macros Patrick Steinhardt
2025-08-04 22:05   ` Taylor Blau
2025-08-04  8:17 ` [PATCH 6/9] commit-graph: store the hash algorithm instead of its length Patrick Steinhardt
2025-08-04 22:07   ` Taylor Blau
2025-08-04  8:17 ` [PATCH 7/9] commit-graph: stop using `the_hash_algo` Patrick Steinhardt
2025-08-04 22:10   ` Taylor Blau
2025-08-06  6:53     ` Patrick Steinhardt
2025-08-04  8:17 ` [PATCH 8/9] commit-graph: stop using `the_repository` Patrick Steinhardt
2025-08-04 22:11   ` Taylor Blau
2025-08-04  8:17 ` [PATCH 9/9] commit-graph: stop passing in redundant repository Patrick Steinhardt
2025-08-05  4:27 ` [PATCH 0/9] commit-graph: remove reliance on global state Derrick Stolee
2025-08-06  6:53   ` Patrick Steinhardt
2025-08-06 12:00 ` [PATCH v2 00/10] " Patrick Steinhardt
2025-08-06 12:00   ` [PATCH v2 01/10] trace2: introduce function to trace unsigned integers Patrick Steinhardt
2025-08-06 12:00   ` [PATCH v2 02/10] commit-graph: stop using signed integers to count Bloom filters Patrick Steinhardt
2025-08-06 12:00   ` [PATCH v2 03/10] commit-graph: fix type for some write options Patrick Steinhardt
2025-08-06 12:34     ` Oswald Buddenhagen
2025-08-06 15:40       ` Junio C Hamano
2025-08-07  7:07         ` Patrick Steinhardt
2025-08-06 12:00   ` [PATCH v2 04/10] commit-graph: fix sign comparison warnings Patrick Steinhardt
2025-08-06 12:00   ` [PATCH v2 05/10] commit-graph: stop using `the_hash_algo` via macros Patrick Steinhardt
2025-08-06 12:00   ` [PATCH v2 06/10] commit-graph: store the hash algorithm instead of its length Patrick Steinhardt
2025-08-06 12:00   ` [PATCH v2 07/10] commit-graph: refactor `parse_commit_graph()` to take a repository Patrick Steinhardt
2025-08-06 12:00   ` [PATCH v2 08/10] commit-graph: stop using `the_hash_algo` Patrick Steinhardt
2025-08-06 12:00   ` [PATCH v2 09/10] commit-graph: stop using `the_repository` Patrick Steinhardt
2025-08-06 12:00   ` [PATCH v2 10/10] commit-graph: stop passing in redundant repository Patrick Steinhardt
2025-08-07  8:04 ` [PATCH v3 00/10] commit-graph: remove reliance on global state Patrick Steinhardt
2025-08-07  8:04   ` [PATCH v3 01/10] trace2: introduce function to trace unsigned integers Patrick Steinhardt
2025-08-07  8:04   ` [PATCH v3 02/10] commit-graph: stop using signed integers to count Bloom filters Patrick Steinhardt
2025-08-07  8:04   ` [PATCH v3 03/10] commit-graph: fix type for some write options Patrick Steinhardt
2025-08-07 22:40     ` Junio C Hamano
2025-08-11  8:24       ` Patrick Steinhardt
2025-08-07  8:04   ` [PATCH v3 04/10] commit-graph: fix sign comparison warnings Patrick Steinhardt
2025-08-07  8:04   ` [PATCH v3 05/10] commit-graph: stop using `the_hash_algo` via macros Patrick Steinhardt
2025-08-07  8:04   ` [PATCH v3 06/10] commit-graph: store the hash algorithm instead of its length Patrick Steinhardt
2025-08-07  8:04   ` [PATCH v3 07/10] commit-graph: refactor `parse_commit_graph()` to take a repository Patrick Steinhardt
2025-08-07  8:04   ` [PATCH v3 08/10] commit-graph: stop using `the_hash_algo` Patrick Steinhardt
2025-08-07  8:04   ` [PATCH v3 09/10] commit-graph: stop using `the_repository` Patrick Steinhardt
2025-08-07  8:04   ` [PATCH v3 10/10] commit-graph: stop passing in redundant repository Patrick Steinhardt
2025-08-15  5:49 ` [PATCH v4 0/6] commit-graph: remove reliance on global state Patrick Steinhardt
2025-08-15  5:49   ` [PATCH v4 1/6] commit-graph: stop using `the_hash_algo` via macros Patrick Steinhardt
2025-08-15  5:49   ` [PATCH v4 2/6] commit-graph: store the hash algorithm instead of its length Patrick Steinhardt
2025-08-15  5:49   ` [PATCH v4 3/6] commit-graph: refactor `parse_commit_graph()` to take a repository Patrick Steinhardt
2025-08-15  5:49   ` [PATCH v4 4/6] commit-graph: stop using `the_hash_algo` Patrick Steinhardt
2025-08-15  5:49   ` [PATCH v4 5/6] commit-graph: stop using `the_repository` Patrick Steinhardt
2025-08-15  5:49   ` [PATCH v4 6/6] commit-graph: stop passing in redundant repository Patrick Steinhardt
2025-08-15 15:17   ` [PATCH v4 0/6] commit-graph: remove reliance on global state Derrick Stolee

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250804-b4-pks-commit-graph-wo-the-repository-v1-4-850d626eb2e8@pks.im \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).