Skip to contentSkip to conversation

Fix a comparison bug in diff-delta.c

PATCH In review Pierre Habouzit 2 participants 3 messages Jun 15, 2016

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Fix a comparison bug in diff-delta.c

(1 << i) < hspace is compared in the int space rather that in the unsigned one. the result will be wrong if hspace is between 0x40000000 and 0x80000000.

---

This is a replacement for <[email protected]> avoiding the ugly cast, and using 1u instead.

 diff-delta.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
diff-delta.c +1 -1
index 7da9205..51e2e56 100644
@@ -152,7 +152,7 @@ struct delta_index * create_delta_index(
 	   initialization in create_delta(). */
 	entries = (bufsize - 1)  / RABIN_WINDOW;
 	hsize = entries / 4;
-	for (i = 4; (1 << i) < hsize && i < 31; i++);
+	for (i = 4; (1u << i) < hsize && i < 31; i++);
 	hsize = 1 << i;
 	hmask = hsize - 1;
 
Signature · 1 line
1.4.1.1
#1 lore raw

Hi,

diff-delta.c struct delta_index * create_delta_index( · line 155

Quoted 2 lines · Pierre Habouzit
diff-delta.c +1 -1
-	for (i = 4; (1 << i) < hsize && i < 31; i++);
+	for (i = 4; (1u << i) < hsize && i < 31; i++);

The variable i never takes on the value 31 (or any higher value), so there is no bug here. Unless you port git to a system where an int has less than 32 bit.

Ciao,
Dscho

#2 lore raw
Quoted 9 lines · Johannes Schindelin

Hi,

diff-delta.c struct delta_index * create_delta_index( · line 155

Quoted 2 lines · Pierre Habouzit
diff-delta.c +1 -1
-	for (i = 4; (1 << i) < hsize && i < 31; i++);
+	for (i = 4; (1u << i) < hsize && i < 31; i++);

The variable i never takes on the value 31 (or any higher value), so there is no bug here. Unless you port git to a system where an int has less than 32 bit.

that remains quite tasteless though, and maybe that patch can be integrated to the "cleanup" series I sent later today.

Signature · 3 lines
·O·  Pierre Habouzit
··O                                                [email protected]
OOO                                                http://www.madism.org