* Re: [GSoC PATCH v2] rm: fix sign comparison warnings
2025-03-16 10:13 ` [GSoC PATCH v2] " Arnav Bhate
@ 2025-03-17 16:47 ` Junio C Hamano
2025-03-17 17:05 ` Arnav Bhate
2025-03-17 17:07 ` [GSoC PATCH v3] " Arnav Bhate
2025-03-17 17:10 ` Arnav Bhate
2 siblings, 1 reply; 15+ messages in thread
From: Junio C Hamano @ 2025-03-17 16:47 UTC (permalink / raw)
To: Arnav Bhate; +Cc: git, Karthik Nayak
Arnav Bhate <bhatearnav@gmail•com> writes:
> -static int get_ours_cache_pos(const char *path, int pos)
> +static int get_ours_cache_pos(const char *path, unsigned int inverted_pos)
This renaming of parameter is not right.
At this point when the value comes to this function, it *IS* the
position, there is nothing inverted about it. It points at the
position in the .cache[] array where an cache_entry at a higher
stage would appear.
It is perfectly fine to state that the value that is returned from
index_name_pos() is potentially inverted. The function is given a
path name (without any stage information) and
- returns a non-negative number, the position in the .cache[] array,
where a cache_entry at stage #0 (i.e. an entry for a path that does
not require conflict resolution), or
- returns a negative number, when there is no such cache_entry
exists. The caller can "invert" the value to recover a position
in the .cache[] array, where a cache_entry for the path at stage
#0 _would_ _have_ been found, if existed. Due to the way the
cache entries are sorted in the .cache[] array, when you are
interested in finding cache entries for a path at higher stages,
like this function is, you can start scanning at this point until
you see an entry for a different path.
Calling the parameter "pos" is the right thing to do. The value
used to come here _could_ have been called "inverted", and the
result of (-inverted_pos-1) can be assigned to "pos". But because
the patch moves the inversion to the caller, what the code in the
while loop sees is no longer "inverted".
> {
> - 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) {
Here is where the caller notices that index_name_pos() did not see a
stage #0 entry. This caller wants to see "ours" entry at stage #2,
so it "inverts" the returned value and asks the helper function if
it sees such an entry in the .cache[] array.
A handful of prerequisite pieces of knowledge to understand this
code are:
- The index (i.e. the .cache[] array) is sorted by full path name
(down from the top level of the working tree).
- The index can have at most one stage #0 entry for each path name.
When a stage #0 entry exists for a path name, there cannot be
higher stage entries (the path is called "resolved").
- The cache entries in the .cache[] array for the same path name
are sorted by their stage number.
- There can be at most one stage #2 entry for each path name, which
are called "ours". Entries at stage #1 are from common ancestor,
entries at stage #3 are from "their" tree. These higher (i.e.
more than zero) stage entries appear only for "conflicting"
paths in the .cache[] array.
With the understanding above, you can see why "our" position is
computed only when index_name_pos() returns negative in this hunk.
> - 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;
The above hunks are perfectly fine.
> @@ -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 &&
OK.
Thanks.
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [GSoC PATCH v2] rm: fix sign comparison warnings
2025-03-17 16:47 ` Junio C Hamano
@ 2025-03-17 17:05 ` Arnav Bhate
0 siblings, 0 replies; 15+ messages in thread
From: Arnav Bhate @ 2025-03-17 17:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Karthik Nayak
Junio C Hamano <gitster@pobox•com> writes:
> Arnav Bhate <bhatearnav@gmail•com> writes:
>
>> -static int get_ours_cache_pos(const char *path, int pos)
>> +static int get_ours_cache_pos(const char *path, unsigned int inverted_pos)
>
> This renaming of parameter is not right.
>
> At this point when the value comes to this function, it *IS* the
> position, there is nothing inverted about it. It points at the
> position in the .cache[] array where an cache_entry at a higher
> stage would appear.
>
> It is perfectly fine to state that the value that is returned from
> index_name_pos() is potentially inverted. The function is given a
> path name (without any stage information) and
>
> - returns a non-negative number, the position in the .cache[] array,
> where a cache_entry at stage #0 (i.e. an entry for a path that does
> not require conflict resolution), or
>
> - returns a negative number, when there is no such cache_entry
> exists. The caller can "invert" the value to recover a position
> in the .cache[] array, where a cache_entry for the path at stage
> #0 _would_ _have_ been found, if existed. Due to the way the
> cache entries are sorted in the .cache[] array, when you are
> interested in finding cache entries for a path at higher stages,
> like this function is, you can start scanning at this point until
> you see an entry for a different path.
>
> Calling the parameter "pos" is the right thing to do. The value
> used to come here _could_ have been called "inverted", and the
> result of (-inverted_pos-1) can be assigned to "pos". But because
> the patch moves the inversion to the caller, what the code in the
> while loop sees is no longer "inverted".
My logic was that it was the inversion of the variable pos, but your
logic makes more sense. I'll make the change.
>> {
>> - 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) {
>
> Here is where the caller notices that index_name_pos() did not see a
> stage #0 entry. This caller wants to see "ours" entry at stage #2,
> so it "inverts" the returned value and asks the helper function if
> it sees such an entry in the .cache[] array.
>
> A handful of prerequisite pieces of knowledge to understand this
> code are:
>
> - The index (i.e. the .cache[] array) is sorted by full path name
> (down from the top level of the working tree).
>
> - The index can have at most one stage #0 entry for each path name.
> When a stage #0 entry exists for a path name, there cannot be
> higher stage entries (the path is called "resolved").
>
> - The cache entries in the .cache[] array for the same path name
> are sorted by their stage number.
>
> - There can be at most one stage #2 entry for each path name, which
> are called "ours". Entries at stage #1 are from common ancestor,
> entries at stage #3 are from "their" tree. These higher (i.e.
> more than zero) stage entries appear only for "conflicting"
> paths in the .cache[] array.
>
> With the understanding above, you can see why "our" position is
> computed only when index_name_pos() returns negative in this hunk.
Thanks for the explanation, I was not able to get this from the code.
>
>> - 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;
>
> The above hunks are perfectly fine.
>
>> @@ -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 &&
>
> OK.
>
> Thanks.
--
Regards,
Arnav Bhate
(He/Him)
^ permalink raw reply [flat|nested] 15+ messages in thread
* [GSoC PATCH v3] rm: fix sign comparison warnings
2025-03-16 10:13 ` [GSoC PATCH v2] " Arnav Bhate
2025-03-17 16:47 ` Junio C Hamano
@ 2025-03-17 17:07 ` Arnav Bhate
2025-03-17 17:12 ` Arnav Bhate
2025-03-17 17:10 ` Arnav Bhate
2 siblings, 1 reply; 15+ messages in thread
From: Arnav Bhate @ 2025-03-17 17:07 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Karthik Nayak
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 &&
--
2.48.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [GSoC PATCH v3] rm: fix sign comparison warnings
2025-03-17 17:07 ` [GSoC PATCH v3] " Arnav Bhate
@ 2025-03-17 17:12 ` Arnav Bhate
0 siblings, 0 replies; 15+ messages in thread
From: Arnav Bhate @ 2025-03-17 17:12 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Karthik Nayak
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.
--
Regards,
Arnav Bhate
(He/Him)
^ permalink raw reply [flat|nested] 15+ messages in thread
* [GSoC PATCH v3] rm: fix sign comparison warnings
2025-03-16 10:13 ` [GSoC PATCH v2] " Arnav Bhate
2025-03-17 16:47 ` Junio C Hamano
2025-03-17 17:07 ` [GSoC PATCH v3] " Arnav Bhate
@ 2025-03-17 17:10 ` Arnav Bhate
2025-03-29 6:03 ` [GSoC PATCH v4] " Arnav Bhate
2 siblings, 1 reply; 15+ messages in thread
From: Arnav Bhate @ 2025-03-17 17:10 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Karthik Nayak
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 &&
--
2.48.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [GSoC PATCH v4] rm: fix sign comparison warnings
2025-03-17 17:10 ` Arnav Bhate
@ 2025-03-29 6:03 ` Arnav Bhate
2025-03-29 6:07 ` Arnav Bhate
0 siblings, 1 reply; 15+ messages in thread
From: Arnav Bhate @ 2025-03-29 6:03 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Karthik Nayak
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..a6565a69cf 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 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 ((pos < the_repository->index->cache_nr) && !strcmp(the_repository->index->cache[pos]->name, path)) {
+ if (ce_stage(the_repository->index->cache[pos]) == 2)
+ return pos;
+ 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 &&
--
2.49.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [GSoC PATCH v4] rm: fix sign comparison warnings
2025-03-29 6:03 ` [GSoC PATCH v4] " Arnav Bhate
@ 2025-03-29 6:07 ` Arnav Bhate
0 siblings, 0 replies; 15+ messages in thread
From: Arnav Bhate @ 2025-03-29 6:07 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Karthik Nayak
I did not notice that the patch I had sent previously had a mistake.
Now, I don't remember how it happened, but I have fixed it. Probably a
mistake in the arguments to git format-patch. This patch should have the
correct name for the variable.
--
Regards,
Arnav Bhate
(He/Him)
^ permalink raw reply [flat|nested] 15+ messages in thread