Re: [GSoC PATCH] rm: fix sign comparison warnings

[Junio C Hamano <gitster@pobox.com>]

Arnav Bhate <bhatearnav@gmail.com> 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.

> @@ -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) ||