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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: "Stefan Beller" <stefanbeller@xxxxxxxxxxxxxx>
The code sequence  ' (1u << i) < hsize && i < 31 ' is a multi step
process, whose first step requires that 'i' is already less that 31,
otherwise the result (1u << i)  is undefined (and  'undef_val < hsize'
can therefore be assumed to be 'false'), and so the later test  i < 31
can always be optimized away as dead code ('i' is already less than 31,
or the short circuit 'and' applies).

So we need to get rid of that code. One way would be to exchange the
order of the conditions, so the expression 'i < 31 && (1u << i) < hsize'
would remove that optimized unstable code already.

However 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 and we can remove that condition
entirely.

Signed-off-by: Stefan Beller <stefanbeller@xxxxxxxxxxxxxx>
Acked-by: Nicolas Pitre <nico@xxxxxxxxxxx>
---

Acked-by: Philip Oakley philipoakley@xxxxxxx

If there are concens that future upgrades may make the (i < 31) test a necessity that it should be added before the &&. If the compiler is able to determine it's dead code in that case then that's OK, at least it would be future proof. Then again it's probably not likely in the near future.


Philip,
thanks for the wording of your mail. I get quickly derailed from the
warnings of the STACK tool and write the other commit messages than
I ought to write. I think the wording of your mail nails it pretty
good, so I put it in the commit message as well.

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.2.g2c2b664


Philip
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]