From: Derrick Stolee Date: Wed, 05 Sep 2018 19:28:26 GMT Subject: Re: [PATCH 04/11] multi-pack-index: verify packname order Message-ID: In-Reply-To: On 9/5/2018 3:14 PM, Stefan Beller wrote: > On Wed, Sep 5, 2018 at 12:11 PM Derrick Stolee wrote: >> On 9/5/2018 2:15 PM, Stefan Beller wrote: >>> On Wed, Sep 5, 2018 at 7:46 AM Derrick Stolee via GitGitGadget >>> wrote: >>>> From: Derrick Stolee >>>> >>>> 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? This is a common occurrence in this series (replacing error() with die()) as we are now exercising those conditions and clarifying what should happen.