All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: marcandre.lureau@redhat.com
Cc: qemu-devel@nongnu.org
Subject: Re: [PATCH v3 1/5] osdep: make readv_writev() work with partial read/write
Date: Thu, 6 Oct 2022 09:31:30 +0100	[thread overview]
Message-ID: <Yz6SYuq3nplb61/a@redhat.com> (raw)
In-Reply-To: <20221006081222.2606746-2-marcandre.lureau@redhat.com>

On Thu, Oct 06, 2022 at 12:12:18PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> With a pipe or other reasons, read/write may return less than the
> requested bytes. This happens with the test-io-channel-command test on
> Windows. glib spawn code uses a binary pipe of 4096 bytes, and the first
> read returns that much (although more are requested), for some unclear
> reason...

Wow, lucky this function isn't too widely used historically!

> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  util/osdep.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

> 
> diff --git a/util/osdep.c b/util/osdep.c
> index 60fcbbaebe..b85715a743 100644
> --- a/util/osdep.c
> +++ b/util/osdep.c

There's a comment just above here that says

   helper function for iov_send_recv

which is not true, as iov_send_recv just calls send/recv instead,
again in a loop with the exact same logic flaw.  Care to fix the
comment and also fix iov_send_recv in the same way ?

> @@ -544,12 +544,17 @@ readv_writev(int fd, const struct iovec *iov, int iov_cnt, bool do_write)
>  {
>      unsigned i = 0;
>      ssize_t ret = 0;
> +    ssize_t off = 0;
>      while (i < iov_cnt) {
>          ssize_t r = do_write
> -            ? write(fd, iov[i].iov_base, iov[i].iov_len)
> -            : read(fd, iov[i].iov_base, iov[i].iov_len);
> +            ? write(fd, iov[i].iov_base + off, iov[i].iov_len - off)
> +            : read(fd, iov[i].iov_base + off, iov[i].iov_len - off);
>          if (r > 0) {
>              ret += r;
> +            off += r;
> +            if (off < iov[i].iov_len) {
> +                continue;
> +            }
>          } else if (!r) {
>              break;
>          } else if (errno == EINTR) {
> @@ -562,6 +567,7 @@ readv_writev(int fd, const struct iovec *iov, int iov_cnt, bool do_write)
>              }
>              break;
>          }
> +        off = 0;
>          i++;
>      }
>      return ret;
> -- 
> 2.37.3
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



  reply	other threads:[~2022-10-06  9:17 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-06  8:12 [PATCH v3 0/5] io/command: implement portable spawn marcandre.lureau
2022-10-06  8:12 ` [PATCH v3 1/5] osdep: make readv_writev() work with partial read/write marcandre.lureau
2022-10-06  8:31   ` Daniel P. Berrangé [this message]
2022-10-06  8:12 ` [PATCH v3 2/5] tests/channel-helper: set blocking in main thread marcandre.lureau
2022-10-06  8:25   ` Daniel P. Berrangé
2022-10-06  8:12 ` [PATCH v3 3/5] io/command: use glib GSpawn, instead of open-coding fork/exec marcandre.lureau
2022-10-06  8:12 ` [PATCH v3 4/5] io/command: implement support for win32 marcandre.lureau
2022-10-06  8:38   ` Daniel P. Berrangé
2022-10-06  8:12 ` [PATCH v3 5/5] tests/unit: make test-io-channel-command work on win32 marcandre.lureau
2022-10-06  8:42   ` Daniel P. Berrangé
2022-10-06  8:46     ` Marc-André Lureau
2022-10-06  8:56       ` Daniel P. Berrangé
2022-10-06 11:26         ` Marc-André Lureau

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=Yz6SYuq3nplb61/a@redhat.com \
    --to=berrange@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /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.