Re: [PATCH] create_delta_index: simplify condition always evaluating to true

From: Nicolas Pitre <nico@fluxnic.net>
Date: Aug 16, 2013, 02:34 UTC
Message-ID: <alpine.LFD.2.03.1308152231230.14472@syhkavp.arg>
In-Reply-To: <7vzjsipr3s.fsf@alter.siamese.dyndns.org>

On Thu, 15 Aug 2013, Junio C Hamano wrote:

Show 12 quoted lines

> Forwarding to the area expert...
> 
> Stefan Beller <stefanbeller@googlemail.com> writes:
> 
> > When checking the previous lines in that function, we can deduct that
> > hsize must always be smaller than (1u<<31), since 506049c7df2c6
> > (fix >4GiB source delta assertion failure), because the entries is
> > capped at an upper bound of 0xfffffffeU, so hsize contains a maximum
> > value of 0x3fffffff, which is smaller than (1u<<31), so i will never
> > be larger than 31.
> >
> > Signed-off-by: Stefan Beller <stefanbeller@googlemail.com>

Acked-by: Nicolas Pitre <nico@fluxnic.net>

You probably could dispense with the comment. The code is obvious enough and the commit log has the rationale already.

Show 29 quoted lines

> > ---
> >  diff-delta.c | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/diff-delta.c b/diff-delta.c
> > index 93385e1..54da95b 100644
> > --- a/diff-delta.c
> > +++ b/diff-delta.c
> > @@ -154,8 +154,15 @@ struct delta_index * create_delta_index(const void *buf, unsigned long bufsize)
> >  		 */
> >  		entries = 0xfffffffeU / RABIN_WINDOW;
> >  	}
> > +
> > +	/*
> > +	 * Do not check i < 31 in the loop, because the assignement
> > +	 * previous to the loop makes sure, hsize is definitely
> > +	 * smaller than 1<<31, hence the loop will always stop
> > +	 * before i exceeds 31 resulting in an infinite loop.
> > +	 */
> >  	hsize = entries / 4;
> > -	for (i = 4; (1u << i) < hsize && i < 31; i++);
> > +	for (i = 4; (1u << i) < hsize; i++);
> >  	hsize = 1 << i;
> >  	hmask = hsize - 1;
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>