Re: [GSoC PATCH] decorate: fix sign comparison warnings

[Junio C Hamano <gitster@pobox.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);


> 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);