All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.