Re: [PATCH 01/19] reftable/stack: stop using `read_in_full()`
- From
Justin Tobler <jltobler@gmail.com>
- Date
- Jan 27, 2025, 16:57 UTC
- Message-ID
- <u7hbqd6at3owgka4pitd4vgzc5emgipnx3w4dat5ofpiv5hxxo@lbfj36uhqtsr>
- In-Reply-To
- <20250127-pks-reftable-drop-git-compat-util-v1-1-6e280a564877@pks.im>
On 25/01/27 02:04PM, Patrick Steinhardt wrote:
Show 38 quoted lines
> There is a single callsite of `read_in_full()` in the reftable library.
> Open-code the function to reduce our dependency on the Git library.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> reftable/stack.c | 18 ++++++++++++++----
> 1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/reftable/stack.c b/reftable/stack.c
> index f7c1845e15..9490366795 100644
> --- a/reftable/stack.c
> +++ b/reftable/stack.c
> @@ -115,13 +115,16 @@ int reftable_new_stack(struct reftable_stack **dest, const char *dir,
>
> static int fd_read_lines(int fd, char ***namesp)
> {
> - off_t size = lseek(fd, 0, SEEK_END);
> char *buf = NULL;
> int err = 0;
> + off_t size;
> +
> + size = lseek(fd, 0, SEEK_END);
> if (size < 0) {
> err = REFTABLE_IO_ERROR;
> goto done;
> }
> +
> err = lseek(fd, 0, SEEK_SET);
> if (err < 0) {
> err = REFTABLE_IO_ERROR;
> @@ -134,9 +137,16 @@ static int fd_read_lines(int fd, char ***namesp)
> goto done;
> }
>
> - if (read_in_full(fd, buf, size) != size) {
> - err = REFTABLE_IO_ERROR;
> - goto done;
> + for (size_t total_read = 0; total_read < (size_t) size; ) {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?
> + 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?
Show 15 quoted lines
> + continue;
> + if (bytes_read < 0 || !bytes_read) {
> + err = REFTABLE_IO_ERROR;
> + goto done;
> + }
> +
> + total_read += bytes_read;
> }
> buf[size] = 0;
>
>
> --
> 2.48.1.362.g079036d154.dirty
>
>