Re: [PATCH 4/9] commit-graph: fix sign comparison warnings
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Aug 6, 2025, 06:52 UTC
- Message-ID
- <aJL7witwAASbQvXZ@pks.im>
- In-Reply-To
- <aJEuZnhxbJcfdWFw@nand.local>
On Mon, Aug 04, 2025 at 06:04:22PM -0400, Taylor Blau wrote:
Show 8 quoted lines
> 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.
Show 10 quoted lines
> > @@ -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