Arnav Bhate <bhatearnav@xxxxxxxxx> 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@xxxxxxxxx>
> ---
> 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.
--
Regards,
Arnav Bhate
(He/Him)
