| Message ID | 306aa7d0-3621-40ba-93c3-77536e56b73a@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [GSoC,v3] rm: fix sign comparison warnings | expand |
Arnav Bhate <bhatearnav@gmail.com> writes: > There are multiple places in loops, where a signed and an > unsigned data type are compared. Git uses a mix of signed and unsigned > types to store lengths of arrays. This sometimes leads to using a signed > index for an array whose length is stored in an unsigned variable or > vice versa. > > get_ours_cache_pos is a special case where i, though derived from a > signed variable is never negative. Move this part to the caller side > and make i an unsigned argument of the function. Rename i to > pos to make it descriptive, now that it is a function argument. > > Replace signed data types with unsigned data types and vice versa > wherever necessary. Where both signed and unsigned data types have been > used, define a new variable in the scope of the for loop for use as the > iterator. Remove #define DISABLE_SIGN_COMPARE_WARNINGS. > > Signed-off-by: Arnav Bhate <bhatearnav@gmail.com> > --- > builtin/rm.c | 21 +++++++++------------ > 1 file changed, 9 insertions(+), 12 deletions(-) > > diff --git a/builtin/rm.c b/builtin/rm.c > index 12ae086a55..a5c9fc644e 100644 > --- a/builtin/rm.c > +++ b/builtin/rm.c > @@ -5,7 +5,6 @@ > */ > > #define USE_THE_REPOSITORY_VARIABLE > -#define DISABLE_SIGN_COMPARE_WARNINGS > > #include "builtin.h" > #include "advice.h" > @@ -40,14 +39,12 @@ static struct { > } *entry; > } list; > > -static int get_ours_cache_pos(const char *path, int pos) > +static int get_ours_cache_pos(const char *path, unsigned int inverted_pos) > { > - int i = -pos - 1; > - > - while ((i < the_repository->index->cache_nr) && !strcmp(the_repository->index->cache[i]->name, path)) { > - if (ce_stage(the_repository->index->cache[i]) == 2) > - return i; > - i++; > + while ((inverted_pos < the_repository->index->cache_nr) && !strcmp(the_repository->index->cache[inverted_pos]->name, path)) { > + if (ce_stage(the_repository->index->cache[inverted_pos]) == 2) > + return inverted_pos; > + inverted_pos++; > } > return -1; > } > @@ -58,7 +55,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); > @@ -83,7 +80,7 @@ static void submodules_absorb_gitdir_if_needed(void) > > pos = index_name_pos(the_repository->index, name, strlen(name)); > if (pos < 0) { > - pos = get_ours_cache_pos(name, pos); > + pos = get_ours_cache_pos(name, -pos - 1); > if (pos < 0) > continue; > } > @@ -131,7 +128,7 @@ static int check_local_mod(struct object_id *head, int index_only) > * Skip unmerged entries except for populated submodules > * that could lose history when removed. > */ > - pos = get_ours_cache_pos(name, pos); > + pos = get_ours_cache_pos(name, -pos - 1); > if (pos < 0) > continue; > > @@ -314,7 +311,7 @@ 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++) { > + for (unsigned int i = 0; i < the_repository->index->cache_nr; i++) { > const struct cache_entry *ce = the_repository->index->cache[i]; > > if (!include_sparse && Please ignore this one, I sent the old patch accidentally.
diff --git a/builtin/rm.c b/builtin/rm.c index 12ae086a55..a5c9fc644e 100644 --- a/builtin/rm.c +++ b/builtin/rm.c @@ -5,7 +5,6 @@ */ #define USE_THE_REPOSITORY_VARIABLE -#define DISABLE_SIGN_COMPARE_WARNINGS #include "builtin.h" #include "advice.h" @@ -40,14 +39,12 @@ static struct { } *entry; } list; -static int get_ours_cache_pos(const char *path, int pos) +static int get_ours_cache_pos(const char *path, unsigned int inverted_pos) { - int i = -pos - 1; - - while ((i < the_repository->index->cache_nr) && !strcmp(the_repository->index->cache[i]->name, path)) { - if (ce_stage(the_repository->index->cache[i]) == 2) - return i; - i++; + while ((inverted_pos < the_repository->index->cache_nr) && !strcmp(the_repository->index->cache[inverted_pos]->name, path)) { + if (ce_stage(the_repository->index->cache[inverted_pos]) == 2) + return inverted_pos; + inverted_pos++; } return -1; } @@ -58,7 +55,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); @@ -83,7 +80,7 @@ static void submodules_absorb_gitdir_if_needed(void) pos = index_name_pos(the_repository->index, name, strlen(name)); if (pos < 0) { - pos = get_ours_cache_pos(name, pos); + pos = get_ours_cache_pos(name, -pos - 1); if (pos < 0) continue; } @@ -131,7 +128,7 @@ static int check_local_mod(struct object_id *head, int index_only) * Skip unmerged entries except for populated submodules * that could lose history when removed. */ - pos = get_ours_cache_pos(name, pos); + pos = get_ours_cache_pos(name, -pos - 1); if (pos < 0) continue; @@ -314,7 +311,7 @@ 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++) { + for (unsigned int i = 0; i < the_repository->index->cache_nr; i++) { const struct cache_entry *ce = the_repository->index->cache[i]; if (!include_sparse &&
There are multiple places in loops, where a signed and an unsigned data type are compared. Git uses a mix of signed and unsigned types to store lengths of arrays. This sometimes leads to using a signed index for an array whose length is stored in an unsigned variable or vice versa. get_ours_cache_pos is a special case where i, though derived from a signed variable is never negative. Move this part to the caller side and make i an unsigned argument of the function. Rename i to pos to make it descriptive, now that it is a function argument. Replace signed data types with unsigned data types and vice versa wherever necessary. Where both signed and unsigned data types have been used, define a new variable in the scope of the for loop for use as the iterator. Remove #define DISABLE_SIGN_COMPARE_WARNINGS. Signed-off-by: Arnav Bhate <bhatearnav@gmail.com> --- builtin/rm.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-)