On Wed, Jul 23, 2025 at 4:06 AM Jeff King <peff@xxxxxxxx> 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.
