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

[Arnav Bhate <bhatearnav@gmail.com>]

Junio C Hamano <gitster@pobox.com> writes:

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

Your suggestion makes sense to me, the second one is better. I will send an
updated patch. I also found some more places in the file where a change from
int to unsigned int should happen, but where int does not cause warnings. I
will also include it in the patch.

-- 
Regards,
Arnav Bhate
(He/Him)