Re: [PATCH 02/13] declare overflow during base128 decoding when 1 MSB nonzero, not 7

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH 02/13] declare overflow during base128 decoding when 1 MSB nonzero, not 7

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:43:03

"Dana How" [off-list ref] writes:

quoted hunk

Subject: [PATCH 02/13] declare overflow during base128 decoding when 1 MSB nonzero, not 7

---
 builtin-pack-objects.c   |    2 +-
 builtin-unpack-objects.c |    2 +-
 index-pack.c             |    2 +-
 sha1_file.c              |    2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index b5f9648..50246e1 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c

@@ -1014,7 +1014,7 @@ static void check_object(struct object_entry *entry)
 				ofs = c & 127;
 				while (c & 128) {
 					ofs += 1;
-					if (!ofs || ofs & ~(~0UL >> 7))
+					if (!ofs || ofs & ~(~0UL >> 1))
 						die("delta base offset overflow in pack for %s",
 						    sha1_to_hex(entry->sha1));
 					c = buf[used_0++];

The line after these context does this:

	ofs = (ofs << 7) + (c & 127);

If you do not check the top 7 bits, wouldn't you miss overflow?

Re: [PATCH 02/13] declare overflow during base128 decoding when 1 MSB nonzero, not 7

From: Dana How <hidden>
Date: 2016-06-15 22:43:03

On 4/5/07, Junio C Hamano [off-list ref] wrote:

"Dana How" [off-list ref] writes:

quoted

                              ofs = c & 127;
                              while (c & 128) {
                                      ofs += 1;
-                                     if (!ofs || ofs & ~(~0UL >> 7))
+                                     if (!ofs || ofs & ~(~0UL >> 1))
                                              die("delta base offset overflow in pack for %s",
                                                  sha1_to_hex(entry->sha1));
                                      c = buf[used_0++];

The line after these context does this:
        ofs = (ofs << 7) + (c & 127);
If you do not check the top 7 bits, wouldn't you miss overflow?

You are correct in all 4 cases --
this patch can be dropped from the set;
 it has no overlap in any context.

Concerning SEEK_SET,
you are correct: my unistd.h #define's SEEK_SET to 0.
But if someone edits this, the arg order might remain.

I will follow your commit text suggestions.

Let me think about your comment on 08/13.

Thanks!
-- 
Dana L. How  danahow@gmail.com  +1 650 804 5991 cell

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help