Re: [PATCH] wrapper: properly handle MAX_IO_SIZE in `write_in_full()`

[Jeff King <peff@peff.net>]

On Thu, Apr 09, 2026 at 09:42:42AM -0700, Junio C Hamano wrote:

> > +	for (i = 0, total_length = 0; i < iovcnt; i++) {
> > +		if (unsigned_add_overflows(total_length, iov[i].iov_len))
> > +			break;
> 
> We add .iov_len up in this first loop, because we do not want to
> bust writev(3p)'s limit.
> 
> 	EINVAL The sum of the iov_len values in the iov array would
>               overflow an ssize_t.
> 
> cf. https://pubs.opengroup.org/onlinepubs/9799919799/functions/writev.html
> 
> As the width of ssize_t in bits can be a lot smaller than size_t,
> the above "unsigned_add_overflows() triggers way too late for the
> check to matter, no?

I think it is correct as-is.

The real check against ssize_t is later, when we compare total_length to
MAX_IO_SIZE (which is clamped to SSIZE_MAX). So this is just making sure
we do not overflow size_t when counting up the total (and if we do, we
_know_ we are going to overflow ssize_t, which must be smaller).

I think this can be made more clear by counting down allowable bytes
instead of up. I'll show an example in a second.

> > +	}
> > +
> > +	if (i < iovcnt) {
> > +		/*
> > +		 * The first entry exceeds MAX_IO_SIZE, so we pass it to
> > +		 * xwrite, which knows to handle this case.
> > +		 */
> > +		if (!i)
> > +			return xwrite(fd, iov->iov_base, iov->iov_len);
> 
> Ben has a similar comment, but it would be easier to see the
> correspondence if you rephrase the comment perhaps like
> [...]

Me three. I think this can be made more clear if we bail to xwrite()
immediately in the loop. So together with the count-down, something
like:

   ssize_t allowed = MAX_IO_SIZE;
   int i;

   for (i = 0; i < iovcnt; i++) {
	if (iov[i].iov_len > allowed) {
		if (!i)
			return xwrite(fd, iov->iov_base, iov_len);
		break;
	}
	allowed -= iov[i].iov_len;
  }
  return writev(fd, iov, i);

You can also directly return writev() instead of breaking out of the
loop. That makes it even more clear that we are doing a partial write,
but means duplicating the "return writev()" line.

And I think the whole thing would still deserve comments, but I omitted
them here since the point was to show the rearranged structure.

-Peff