Subject: Re: [PATCH 01/19] reftable/stack: stop using `read_in_full()`

From: Junio C Hamano <[email-masked]>
Date: Tue, 28 Jan 2025 09:05:44 -0800

Patrick Steinhardt <[email-masked]> writes:

>> The cast from off_t -> size_t matches the currect behavior, but is it
>> always safe to do this? In `git-compat-util.h` it looks like we have
>> `xsize_t()` to safely handle these conversions. Since this series is
>> moving away from `git-compat-util.h` should ideally something similar be
>> implemented?
>
> It is safe, because a couple lines further up we check for `size < 0`
> and error out if that is the case. So we know it's a positive integer,
> and thus it can be represented via `size_t`.

Even where off_t (which measures on-disk file in bytes) may be wider
than size_t (which measures in-core piece of memory in bytes)?

>> > +		ssize_t bytes_read = read(fd, buf + total_read, size - total_read);
>> > +		if (bytes_read < 0 && (errno == EAGAIN || errno == EINTR))
>> 
>> The error handling here for EAGAIN doesn't go as far as what `xread()`
>> does via `handle_nonblock()`. In this scenario is that ok?
>
> Yes, because we don't set `O_NONBLOCK` in the reftable library.
>
> I'll note that in the commit message.

Good.  Thanks.