diff mbox series

[2/9] commit-reach: fix index used to loop through unsigned integer

Message ID 20241227-b4-pks-commit-reach-sign-compare-v1-2-07c59c2aa632@pks.im (mailing list archive)
State Accepted
Commit 95c09e4d07492fa9e4ad951a268b4ea6bae69038
Headers show
Series commit-reach: -Wsign-compare follow-ups | expand

Commit Message

Patrick Steinhardt Dec. 27, 2024, 10:46 a.m. UTC
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 <ps@pks.im>
---
 commit-reach.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Jeff King Dec. 27, 2024, 2:46 p.m. UTC | #1
On Fri, Dec 27, 2024 at 11:46:22AM +0100, Patrick Steinhardt wrote:

> 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.

Yes, that's exactly what happened (I used a merge of my series and your
-Wsign-compare one and relied on the compiler).

I grepped through for other instances and couldn't find any problems.
Many places just do:

  while (queue->nr)
     ...remove top...

which is fine with either type (which might explain why the problem
wasn't more common).

I thought there was one more instance in commit-graph.c, which looks at
"info->commits->nr", and I was grepping for "commits->nr" since
rev_info->commits is a prio_queue. But it is actually a totally
different type, compute_generation_info, and "commits" there is a
packed_commit_list.

That data type _also_ has the same int/size_t problem. ;) But it is
totally separate, so it can happen later in its own patch (and there are
a ton of other -Wsign-compare warnings in that file).

Thanks for cleaning up after me.

-Peff
diff mbox series

Patch

diff --git a/commit-reach.c b/commit-reach.c
index e3edd1199529792fafbaa03999c5b7b202f7cf1b..e65872617003d0e43776909c30343f091d6b42f2 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;