{"thread":{"id":"5359","subject":"[PATCH] Fix a comparison bug in diff-delta.c","startedAt":"2006-08-23T02:32:48Z","lastAt":"2006-08-23T03:17:24Z","messageCount":3,"participants":["Pierre Habouzit","Nicolas Pitre"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"25778","messageId":"1156300368160-git-send-email-madcoder@debian.org","threadId":"5359","inReplyTo":null,"subject":"[PATCH] Fix a comparison bug in diff-delta.c","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2006-08-23T02:32:48Z","receivedAt":"2006-08-23T02:32:48Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"(1 << i) < hspace is compared in the `int` space rather that in the unsigned one.\nthe result will be wrong if hspace is between 0x40000000 and 0x80000000.\n\nSigned-off-by: Pierre Habouzit \n---\n\n I'm currently trying to make git compile with more strict gcc flags\n(-g -O2 -Wall -Wextra -Wno-unused -Werror to be precise) and I've\nspotted a first bug due to a signed/unsigned comparison.\n\n If I do understand that bit of code, it should not bite a lot of\npeople, but this is still a bug ;)\n\n\n diff-delta.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/diff-delta.c b/diff-delta.c\nindex 7da9205..a1fadc9 100644\n--- a/diff-delta.c\n+++ b/diff-delta.c\n@@ -152,7 +152,7 @@ struct delta_index * create_delta_index(\n \t initialization in create_delta(). */\n \tentries = (bufsize - 1) / RABIN_WINDOW;\n \thsize = entries / 4;\n-\tfor (i = 4; (1 << i) < hsize && i < 31; i++);\n+\tfor (i = 4; (unsigned)(1 << i) < hsize && i < 31; i++);\n \thsize = 1 << i;\n \thmask = hsize - 1;\n \n-- \n1.4.1.1\n"},{"id":"25779","messageId":"200608230438.06053.madcoder@debian.org","threadId":"5359","inReplyTo":"1156300368160-git-send-email-madcoder@debian.org","subject":"Re: [PATCH] Fix a comparison bug in diff-delta.c","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2006-08-23T02:38:02Z","receivedAt":"2006-08-23T02:38:02Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"Le mer 23 août 2006 04:32, Pierre Habouzit a écrit :\n>   I'm currently trying to make git compile with more strict gcc flags\n> (-g -O2 -Wall -Wextra -Wno-unused -Werror to be precise) and I've\n> spotted a first bug due to a signed/unsigned comparison.\n\nstill in the same kind of \"problems\" sha1_name.c:512 has:\n\n\t\tif (active_nr < 0)\n\t\t\treturn -1;\n\nbut active_nr is defined as an unsigned (in cache.h/cache-read.c).\n\nAlso note that there is a *lot* of signed/unsigned comparisons due to \nthe fact that it's unsigned, and that it's often compared to positions \nthat are ints (I assume pos need the negative values to mean \"error\").\n\nso either those two lines are too much, or active_nr has to be a plain \nsigned int. I'm not used to git sources enough yet to be able to make \nthe right decision.\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"25781","messageId":"Pine.LNX.4.64.0608222311060.3683@localhost.localdomain","threadId":"5359","inReplyTo":"1156300368160-git-send-email-madcoder@debian.org","subject":"Re: [PATCH] Fix a comparison bug in diff-delta.c","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2006-08-23T03:17:24Z","receivedAt":"2006-08-23T03:17:24Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Wed, 23 Aug 2006, Pierre Habouzit wrote:\n\n> (1 << i) < hspace is compared in the `int` space rather that in the unsigned one.\n> the result will be wrong if hspace is between 0x40000000 and 0x80000000.\n> \n> Signed-off-by: Pierre Habouzit \n\nCould you use (1u << i) instead of (unsigned)(1 << i) ? That looks \nprettier to me at least.\n\n\nNicolas\n"}]}