Re: [PATCH 04/10] unpack_loose_header(): fix infinite loop on broken zlib input

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Feb 25, 2025, 19:00 UTC
Message-ID: <CAPig+cSq89192SvKQRcPqDTnBEkVtLyrVMPmU989c5gwkO1tUw@mail.gmail.com>
In-Reply-To: <Z72sp639o8rxG1Qf@pks.im>

On Tue, Feb 25, 2025 at 6:46 AM Patrick Steinhardt <ps@pks.im> wrote:

Show 7 quoted lines

> On Tue, Feb 25, 2025 at 01:29:58AM -0500, Jeff King wrote:
> > +     mv "$objpath" obj.bak &&
> > +     test_when_finished 'mv obj.bak "$objpath"' &&
>
> The order should probably be reversed here, as we nowadays tend to first
> queue the cleanup before doing the actual work. Not that it really
> matters in this case.

I'd say this case is fine as-is since that particular cleanup *only* makes sense if `mv` succeeded. Moreover, if these statements were to be reversed, we'd need to take extra precaution (using `||:`, for instance) against pointless failure of the cleanup code itself; i.e.:

    test_when_finished 'mv obj.bak "$objpath" ||:' &&
    mv "$objpath" obj.bak &&