Re: [GSoC PATCH] rm: fix sign comparison warnings
Junio C Hamano <gitster@pobox.com> writes:
Show 19 quoted lines
> 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.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.
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) ||--
Regards,
Arnav Bhate
(He/Him)
Arnav Bhate <bhatearnav@gmail.com>