git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stefan Beller <stefanbeller@googlemail.com>
To: sunshine@sunshineco.com, git@vger.kernel.org, gitster@pobox.com
Cc: Stefan Beller <stefanbeller@googlemail.com>
Subject: [PATCH] create_delta_index: simplify condition always evaluating to true
Date: Thu, 15 Aug 2013 23:34:22 +0200	[thread overview]
Message-ID: <1376602462-32339-1-git-send-email-stefanbeller@googlemail.com> (raw)
In-Reply-To: <CAPig+cQmdPo4mo69DsDmUURcw+HbxkAoNEqY08qiuJs8S+=bvQ@mail.gmail.com>

When checking the previous lines in that function, we can deduce that
hsize must always be smaller than (1u<<31), since 506049c7df2c6
(fix >4GiB source delta assertion failure), because entries is
capped at an upper bound of 0xfffffffeU, so hsize contains a maximum
value of 0x3fffffff, which is smaller than (1u<<31), so the value of
'i' will never be larger than 31.

Signed-off-by: Stefan Beller <stefanbeller@googlemail.com>
---

Eric, thanks for reviewing my patch.

I applied the first 2 proposals (deduce, entries), but I disagree on
the third, so I reformulated the sentence, as I really meant the variable
i and not it as a pronoun.

Do I understand right, you're suggesting to remove the 
source code comment? I did this now, but I have a bad feeling with it.

The change of this patch surely removes dead code as of now and makes it
more readable. But also it could become alive again, once somebody
changes things nearby and forgets about the assumption, hsize not
exceeding a certain size. That's why I put a comment in there, so 
the future changes nearby may be more careful.

Thanks,
Stefan


 diff-delta.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/diff-delta.c b/diff-delta.c
index 93385e1..3797ce6 100644
--- a/diff-delta.c
+++ b/diff-delta.c
@@ -155,7 +155,7 @@ struct delta_index * create_delta_index(const void *buf, unsigned long bufsize)
 		entries = 0xfffffffeU / RABIN_WINDOW;
 	}
 	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;
 
-- 
1.8.4.rc3.1.gc1ebd90

  reply	other threads:[~2013-08-15 21:34 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-15 19:37 [PATCH] create_delta_index: simplify condition always evaluating to true Stefan Beller
2013-08-15 21:21 ` Eric Sunshine
2013-08-15 21:34   ` Stefan Beller [this message]
2013-08-15 21:46     ` Eric Sunshine
2013-08-16  2:38       ` Nicolas Pitre
2013-08-16 16:47       ` Philip Oakley
2013-08-16 21:22         ` Stefan Beller
2013-08-17  6:58           ` Philip Oakley
2013-08-15 22:14     ` Philip Oakley
2013-08-15 21:43 ` Junio C Hamano
2013-08-15 22:04   ` Stefan Beller
2013-08-16  2:34   ` Nicolas Pitre
2013-08-16 11:43 ` brian m. carlson
2013-08-16 12:40   ` Eric Sunshine

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1376602462-32339-1-git-send-email-stefanbeller@googlemail.com \
    --to=stefanbeller@googlemail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).