Re: [GSoC PATCH] rm: fix sign comparison warnings
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 13, 2025, 07:16 UTC
- Message-ID
- <xmqqy0x9s8mg.fsf@gitster.g>
- In-Reply-To
- <38de63ce-6d4e-4f1f-95b1-049df78d9cfc@gmail.com>
Arnav Bhate <bhatearnav@gmail.com> writes:
Show 9 quoted lines
> static int get_ours_cache_pos(const char *path, int pos)
> {
> - int i = -pos - 1;
> + /*
> + * This function is only called when pos < 0, so -pos - 1 is
> + * greater than or equal to 0, so it can be safely be stored in
> + * an unsigned int.
> + */
> + unsigned int i = -pos - 1;"Can be safely stored", sure.
But so is "int i" perfectly adequate to hold such a value, no?
This is one of the many instances that demonstrate why the "-Wsign-compare" warning is of dubious value, and invites worse code than necessary.
Show 28 quoted lines
> @@ -58,7 +62,7 @@ static void print_error_files(struct string_list *files_list,
> int *errs)
> {
> if (files_list->nr) {
> - int i;
> + unsigned int i;
> struct strbuf err_msg = STRBUF_INIT;
>
> strbuf_addstr(&err_msg, main_msg);
> @@ -271,6 +275,7 @@ int cmd_rm(int argc,
> {
> struct lock_file lock_file = LOCK_INIT;
> int i, ret = 0;
> + unsigned int j;
> struct pathspec pathspec;
> char *seen;
>
> @@ -314,8 +319,8 @@ int cmd_rm(int argc,
> if (pathspec_needs_expanded_index(the_repository->index, &pathspec))
> ensure_full_index(the_repository->index);
>
> - for (i = 0; i < the_repository->index->cache_nr; i++) {
> - const struct cache_entry *ce = the_repository->index->cache[i];
> + for (j = 0; j < the_repository->index->cache_nr; j++) {
> + const struct cache_entry *ce = the_repository->index->cache[j];
>
> if (!include_sparse &&
> (ce_skip_worktree(ce) ||