Junio C Hamano <gitster@xxxxxxxxx> writes: > Arnav Bhate <bhatearnav@xxxxxxxxx> writes: > >> 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. While "int i" may be adequate, this, in my opinion, emphasises the fact that we do not want i to take negative values, so I do not think this is worse. Karthik's suggestion in another reply is also an alternative. >> @@ -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) || -- Regards, Arnav Bhate (He/Him)