Re: [PATCH v3 2/2] t/helper/test-delta: fix possible resource leak and ensure safe cleanup

[Eric Sunshine <sunshine@sunshineco.com>]

On Wed, Jul 23, 2025 at 4:06 AM Jeff King <peff@peff.net> wrote:
> On Wed, Jul 23, 2025 at 03:55:13AM -0400, Jeff King wrote:
> > I dunno. We are reaching diminishing returns spending brainpower on a
> > function that is meant to be somewhat quick-and-dirty.
>
> OK, I clearly could not resist spending more brainpower on it. If we are
> doing quick-and-dirty, why not just die()? The end result is the same,
> but per my argument in the earlier iteration of the series, that means
> we do not have to worry about cleaning up at all.

Yes, die() seems sensible here. It's nice and tidy and makes the code
easier to reason about.

> Incidentally that would also fix two minor bugs I noticed:
>
>   - passing st.st_size directly to xmalloc() is wrong, because of
>     truncation from off_t to size_t. This should use the xsize_t helper.
>     This is even a potential security vulnerability, but probably not
>     important in a test helper.
>
>   - likewise read_in_full() might return a non-negative value smaller
>     than the requested size (if the file racily changes and we get an
>     early EOF). But we only check whether we got a negative error value.
>     So we may read fewer bytes than expected and feed uninitialized
>     garbage to the delta code.

Can of worms opened.