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

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Aug 15, 2013, 21:21 UTC
Message-ID: <CAPig+cQmdPo4mo69DsDmUURcw+HbxkAoNEqY08qiuJs8S+=bvQ@mail.gmail.com>
In-Reply-To: <1376595460-6546-1-git-send-email-stefanbeller@googlemail.com>

On Thu, Aug 15, 2013 at 3:37 PM, Stefan Beller <stefanbeller@googlemail.com> wrote:

> When checking the previous lines in that function, we can deduct that

s/deduct/deduce/

> hsize must always be smaller than (1u<<31), since 506049c7df2c6
> (fix >4GiB source delta assertion failure), because the entries is

s/the entries/entries/ reads a bit better.

> 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

s/i/it/

Show 22 quoted lines

> be larger than 31.
>
> Signed-off-by: Stefan Beller <stefanbeller@googlemail.com>
> ---
>  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.
> +        */

This comment echoes the commit message, and indeed the explanation makes more sense in that context since someone can read the entire patch to see and understand the actual change. It may have less value as an in-code comment.

Show 7 quoted lines

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