Re: [PATCH 04/11] multi-pack-index: verify packname order

From: Stefan Beller <sbeller@google.com>
Date: Sep 5, 2018, 19:14 UTC
Message-ID: <CAGZ79kZHmCyRLur=DgLx2sHhD1hCXn3pU+0CggwFQgWRzTkp5A@mail.gmail.com>
In-Reply-To: <da7eaf92-57f6-8129-149d-4260d84b4eb0@gmail.com>

On Wed, Sep 5, 2018 at 12:11 PM Derrick Stolee <stolee@gmail.com> wrote:

Show 22 quoted lines

>
> On 9/5/2018 2:15 PM, Stefan Beller wrote:
> > On Wed, Sep 5, 2018 at 7:46 AM Derrick Stolee via GitGitGadget
> > <gitgitgadget@gmail.com> wrote:
> >> From: Derrick Stolee <dstolee@microsoft.com>
> >>
> >> The final check we make while loading a multi-pack-index is that
> >> the packfile names are in lexicographical order. Make this error
> >> be a die() instead.
> > What is the advantage of having a die() here?
> > Would the midx work under normal operation when
> > not sorted correctly?
> >
> > This sounds like a hack for easy testability in this context,
> > so could you clarify why this helps the regular user?
>
> The multi-pack-index will not work as expected, since
> midx_contains_pack() may provide incorrect results, and thus we will add
> the "missing" packfiles to the packed_git linked list.
>
> This _should_ be a die(), as something unexpected occurred (the file
> doesn't match the format specification).

Thanks for the clarification.

So this patch actually hits 2 birds with one stone, as it fixes a bug as well as adds the check for such corruption to the verify step?

Thanks, Stefan