Subject: Re: [PATCH 4/9] commit-graph: fix sign comparison warnings

From: Patrick Steinhardt <[email-masked]>
Date: Wed, 6 Aug 2025 08:52:50 +0200

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