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

From: Dana How <danahow@gmail.com>
Date: Apr 5, 2007, 23:08 UTC
Message-ID: <56b7f5510704051608g4bf9ffb7t9f94b532c7a0ebdf@mail.gmail.com>
In-Reply-To: <7vr6qymo3r.fsf@assigned-by-dhcp.cox.net>

On 4/5/07, Junio C Hamano <junkio@cox.net> wrote:

Show 12 quoted lines

> "Dana How" <danahow@gmail.com> writes:
> >                               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