Git development
 help / color / mirror / Atom feed
From: Phillip Wood <phillip.wood123@gmail.com>
To: Patrick Steinhardt <ps@pks.im>,
	"brian m. carlson" <sandals@crustytoothpaste.net>,
	Jeff King <peff@peff.net>, Junio C Hamano <gitster@pobox.com>,
	rsbecker@nexbridge.com, git@vger.kernel.org
Subject: Re: Git 2.54.0-rc1, subtests of t5310, t5326, t5327
Date: Thu, 9 Apr 2026 10:48:02 +0100	[thread overview]
Message-ID: <90c6112d-6447-45e0-8d15-a0a3f1f25013@gmail.com> (raw)
In-Reply-To: <addgkjiB80pgKw69@pks.im>

On 09/04/2026 09:17, Patrick Steinhardt wrote:
> 
> One question to Randall though: does MAX_IO_SIZE apply to the overall
> size of the iovec or to the individual iovec entries? 

In <014e01dcc793$8a9bab90$9fd302b0$@nexbridge.com> Randall says

     Specifying  the sum of the iov_len values in the iov array greater
     than the OSS I/O size limit for that open causes the  writev()
     function  to return  -1  and  set errno to [EINVAL].

So it is the overall size which fits with POSIX limiting to overall size 
to SSIZE_MAX.

Thanks

Phillip

> I think it should
> be the latter, but I cannot easily verify and couldn't find any docs
> around this. So could you please try the patch at the end of this mail
> to verify that it works on your system?
> 
> In any case, I've tested that my patch also works when defining
> MAX_IO_SIZE to 128 bytes on my system, which hopefully demonstrates that
> it works as expected:
> 
> diff --git a/git-compat-util.h b/git-compat-util.h
> index 4b4ea2498f..8e02b5f673 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -690,14 +690,8 @@ static inline uint64_t u64_add(uint64_t a, uint64_t b)
>    * to override this, if the definition of SSIZE_MAX given by the platform
>    * is broken.
>    */
> -#ifndef MAX_IO_SIZE
> -# define MAX_IO_SIZE_DEFAULT (8*1024*1024)
> -# if defined(SSIZE_MAX) && (SSIZE_MAX < MAX_IO_SIZE_DEFAULT)
> -#  define MAX_IO_SIZE SSIZE_MAX
> -# else
> -#  define MAX_IO_SIZE MAX_IO_SIZE_DEFAULT
> -# endif
> -#endif
> +#undef MAX_IO_SIZE
> +#define MAX_IO_SIZE 128
>   
>   #ifdef HAVE_ALLOCA_H
>   # include <alloca.h>
> 
> I'm happy to go either way, but think that we should definitely aim for
> the below patch eventually. Just let me know which way you prefer and
> I'm happy to polish up the patch.
> 
> Patrick
> 
> diff --git a/wrapper.c b/wrapper.c
> index be8fa575e6..645dbc5f20 100644
> --- a/wrapper.c
> +++ b/wrapper.c
> @@ -323,21 +323,50 @@ ssize_t write_in_full(int fd, const void *buf, size_t count)
>   	return total;
>   }
>   
> +ssize_t xwritev(int fd, struct iovec *iov, int iovcnt)
> +{
> +	ssize_t bytes_written;
> +	int i;
> +
> +	/*
> +	 * We need to make sure that no individual iovec entry exceeds
> +	 * `MAX_IO_SIZE`. If there's any entry that does exceed this limit
> +	 * we'll pass all entries up to it to `writev()`, and then process the
> +	 * exceeding entry via a call to `xwrite()`.
> +	 */
> +	for (i = 0; i < iovcnt; i++)
> +		if (iov[i].iov_len > MAX_IO_SIZE)
> +			break;
> +	if (i < iovcnt) {
> +		/*
> +		 * The first entry exceeds MAX_IO_SIZE, so we pass it to
> +		 * xwrite, which knows to handle his case.
> +		 */
> +		if (!i)
> +			return xwrite(fd, iov->iov_base, iov->iov_len);
> +		iovcnt = i;
> +	}
> +
> +	bytes_written = writev(fd, iov, iovcnt);
> +	if (!bytes_written) {
> +		errno = ENOSPC;
> +		return -1;
> +	}
> +
> +	return bytes_written;
> +}
> +
>   ssize_t writev_in_full(int fd, struct iovec *iov, int iovcnt)
>   {
>   	ssize_t total_written = 0;
>   
>   	while (iovcnt) {
> -		ssize_t bytes_written = writev(fd, iov, iovcnt);
> -		if (bytes_written < 0) {
> +		ssize_t bytes_written = xwritev(fd, iov, iovcnt);
> +		if (bytes_written <= 0) {
>   			if (errno == EINTR || errno == EAGAIN)
>   				continue;
>   			return -1;
>   		}
> -		if (!bytes_written) {
> -			errno = ENOSPC;
> -			return -1;
> -		}
>   
>   		total_written += bytes_written;
>   
> diff --git a/wrapper.h b/wrapper.h
> index 27519b32d1..a6287d7f4d 100644
> --- a/wrapper.h
> +++ b/wrapper.h
> @@ -16,6 +16,7 @@ void *xmmap_gently(void *start, size_t length, int prot, int flags, int fd, off_
>   int xopen(const char *path, int flags, ...);
>   ssize_t xread(int fd, void *buf, size_t len);
>   ssize_t xwrite(int fd, const void *buf, size_t len);
> +ssize_t xwritev(int fd, struct iovec *iov, int iovcnt);
>   ssize_t xpread(int fd, void *buf, size_t len, off_t offset);
>   int xdup(int fd);
>   FILE *xfopen(const char *path, const char *mode);
> 


  reply	other threads:[~2026-04-09  9:48 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-07 23:29 Git 2.54.0-rc1, subtests of t5310, t5326, t5327 rsbecker
2026-04-08  4:17 ` Jeff King
2026-04-08 14:54   ` rsbecker
2026-04-08 16:25     ` rsbecker
2026-04-08 17:39       ` Jeff King
2026-04-08 18:12         ` Junio C Hamano
2026-04-08 20:08           ` rsbecker
2026-04-08 20:21             ` Junio C Hamano
2026-04-08 21:27               ` rsbecker
2026-04-08 21:43                 ` Junio C Hamano
2026-04-08 22:04                   ` rsbecker
2026-04-08 22:24                   ` Junio C Hamano
2026-04-08 22:35                     ` Junio C Hamano
2026-04-08 23:15                       ` rsbecker
2026-04-08 22:32                   ` Jeff King
2026-04-09  0:20                     ` brian m. carlson
2026-04-09  8:17                       ` Patrick Steinhardt
2026-04-09  9:48                         ` Phillip Wood [this message]
2026-04-09 11:29                           ` Patrick Steinhardt
2026-04-09 13:46                         ` rsbecker
2026-04-09 20:33                           ` Jeff King
2026-04-09 22:40                             ` rsbecker
2026-04-09 22:58                               ` Jeff King
2026-04-10  4:34                                 ` Patrick Steinhardt
2026-04-09 20:51                         ` Jeff King
2026-04-10  7:35                         ` Johannes Sixt
2026-04-08 18:36         ` rsbecker
2026-04-08 22:14           ` Jeff King
2026-04-08 17:37     ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=90c6112d-6447-45e0-8d15-a0a3f1f25013@gmail.com \
    --to=phillip.wood123@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=phillip.wood@dunelm.org.uk \
    --cc=ps@pks.im \
    --cc=rsbecker@nexbridge.com \
    --cc=sandals@crustytoothpaste.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox