Re: [GSoC PATCH] decorate: fix sign comparison warnings
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 10, 2025, 16:40 UTC
- Message-ID
- <xmqqikog96uz.fsf@gitster.g>
- In-Reply-To
- <afa6b428-3190-42ae-9eac-540c95b576fd@gmail.com>
Arnav Bhate <bhatearnav@gmail.com> writes:
> In two instances, an int was initialized and assigned the value of an > unsigned int. Then, the int was compared to unsigned ints. > > Replace int with unsigned int in both cases.
And these places do not use a negative value to mean anything special.
A simpler fix to the first hunk may be to get rid of the intermediate variable altogether and always refer to n->size when its value is needed. The compiler should be able to see in this static file-scope helper function that n->size would not change at all and do the right thing (i.e. allocate a register to hold its value at entry, if needed) without a hand-optimization we see in the original code.
The same can be said for the second hunk. The intermediate variable is used only once, and one could argue that its presense obscures the condition under which grow_decoration() is called by splitting a logically single expression into two.
Which one is easier to grok?
unsigned nr = n->nr + 1; if (nr > n->size * 2 / 3) grow_decoration(n);
or if ((n->nr + 1) > n->size * 2 / 3) grow_decoration(n);
Show 36 quoted lines
> Signed-off-by: Arnav Bhate <bhatearnav@gmail.com>
> ---
> decorate.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/decorate.c b/decorate.c
> index e161e13772..8d5774fcdd 100644
> --- a/decorate.c
> +++ b/decorate.c
> @@ -3,8 +3,6 @@
> * data.
> */
>
> -#define DISABLE_SIGN_COMPARE_WARNINGS
> -
> #include "git-compat-util.h"
> #include "object.h"
> #include "decorate.h"
> @@ -16,7 +14,7 @@ static unsigned int hash_obj(const struct object *obj, unsigned int n)
>
> static void *insert_decoration(struct decoration *n, const struct object *base, void *decoration)
> {
> - int size = n->size;
> + unsigned int size = n->size;
> struct decoration_entry *entries = n->entries;
> unsigned int j = hash_obj(base, size);
>
> @@ -59,7 +57,7 @@ static void grow_decoration(struct decoration *n)
> void *add_decoration(struct decoration *n, const struct object *obj,
> void *decoration)
> {
> - int nr = n->nr + 1;
> + unsigned int nr = n->nr + 1;
>
> if (nr > n->size * 2 / 3)
> grow_decoration(n);