git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: Taylor Blau <me@ttaylorr.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 4/9] commit-graph: fix sign comparison warnings
Date: Wed, 6 Aug 2025 08:52:50 +0200	[thread overview]
Message-ID: <aJL7witwAASbQvXZ@pks.im> (raw)
In-Reply-To: <aJEuZnhxbJcfdWFw@nand.local>

On Mon, Aug 04, 2025 at 06:04:22PM -0400, Taylor Blau wrote:
> On Mon, Aug 04, 2025 at 10:17:20AM +0200, Patrick Steinhardt wrote:
> > 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`.
> 
> I have similar thoughts as in the previous patch about this spot, too.

I'll convert these to be `uint32_t` instead of `size_t`. We already use
that type to iterate through these counters anyway.

> > @@ -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) {
> 
> I understand why the compiler is telling you to make hexsz a signed
> quantity, but I am not sure that the cast here is aiding the reader, nor
> am I sure that it is making the code safer.

No, it definitely isn't. What makes the code safer is that from now on
we'll get warnings about signedness mismatches.

Patrick

  reply	other threads:[~2025-08-06  6:52 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 ` [PATCH 4/9] commit-graph: fix sign comparison warnings Patrick Steinhardt
2025-08-04 22:04   ` Taylor Blau
2025-08-06  6:52     ` Patrick Steinhardt [this message]
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=aJL7witwAASbQvXZ@pks.im \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    --cc=me@ttaylorr.com \
    /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).