[Qemu-devel] [PATCH] iov_send_recv(): Handle zero bytes case even if OS does not

[Qemu-devel] [PATCH] iov_send_recv(): Handle zero bytes case even if OS does not

[Peter Maydell]

POSIX allows sendmsg() and recvmsg() to fail EMSGSIZE if passed a zero
msg.msg_iovlen (in particular the MacOS X implementation will do this).
Handle the case where iov_send_recv() is passed a zero byte count
explicitly, to avoid accidentally depending on the OS to treat zero
msg_iovlen as a no-op.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
This is what was causing 'make check' to fail on MacOS X.
The other option was to declare that a zero bytecount was illegal, I guess.

 iov.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/iov.c b/iov.c
index b333061..60705c7 100644
--- a/iov.c
+++ b/iov.c
@@ -146,6 +146,13 @@ ssize_t iov_send_recv(int sockfd, struct iovec *iov, unsigned iov_cnt,
 {
     ssize_t ret;
     unsigned si, ei;            /* start and end indexes */
+    if (bytes == 0) {
+        /* Catch the do-nothing case early, as otherwise we will pass an
+         * empty iovec to sendmsg/recvmsg(), and not all implementations
+         * accept this.
+         */
+        return 0;
+    }
 
     /* Find the start position, skipping `offset' bytes:
      * first, skip all full-sized vector elements, */
-- 
1.7.11.4

Re: [Qemu-devel] [PATCH] iov_send_recv(): Handle zero bytes case even if OS does not

[Michael Tokarev]

On 12.08.2012 01:24, Peter Maydell wrote:
> POSIX allows sendmsg() and recvmsg() to fail EMSGSIZE if passed a zero
> msg.msg_iovlen (in particular the MacOS X implementation will do this).

> Handle the case where iov_send_recv() is passed a zero byte count
> explicitly, to avoid accidentally depending on the OS to treat zero
> msg_iovlen as a no-op.

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> This is what was causing 'make check' to fail on MacOS X.
> The other option was to declare that a zero bytecount was illegal, I guess.

I don't think sending/receiving zero bytes is a good idea in the
first place.  Which test were failed on MacOS?  Was it failing at
test-iov "random I/O"?

I thought I ensured that the test does not call any i/o function
with zero "count" argument.  Might be I was wrong, and in that
case THAT place should be fixed instead.

Can you provide a bit more details please?

The whole thing is actually interesting: this is indeed a system-
dependent corner case which should be handled in the code to make
the routine consistent.  But how to fix this is an open question
I think.  Your approach seems to be best, but we as well may
print a warning there...

Thank you!

/mjt

>  iov.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/iov.c b/iov.c
> index b333061..60705c7 100644
> --- a/iov.c
> +++ b/iov.c
> @@ -146,6 +146,13 @@ ssize_t iov_send_recv(int sockfd, struct iovec *iov, unsigned iov_cnt,
>  {
>      ssize_t ret;
>      unsigned si, ei;            /* start and end indexes */
> +    if (bytes == 0) {
> +        /* Catch the do-nothing case early, as otherwise we will pass an
> +         * empty iovec to sendmsg/recvmsg(), and not all implementations
> +         * accept this.
> +         */
> +        return 0;
> +    }
>  
>      /* Find the start position, skipping `offset' bytes:
>       * first, skip all full-sized vector elements, */

Re: [Qemu-devel] [PATCH] iov_send_recv(): Handle zero bytes case even if OS does not

[Peter Maydell]

On 12 August 2012 06:29, Michael Tokarev <mjt@tls.msk.ru> wrote:
> On 12.08.2012 01:24, Peter Maydell wrote:
>> POSIX allows sendmsg() and recvmsg() to fail EMSGSIZE if passed a zero
>> msg.msg_iovlen (in particular the MacOS X implementation will do this).
>
>> Handle the case where iov_send_recv() is passed a zero byte count
>> explicitly, to avoid accidentally depending on the OS to treat zero
>> msg_iovlen as a no-op.
>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>> This is what was causing 'make check' to fail on MacOS X.
>> The other option was to declare that a zero bytecount was illegal, I guess.
>
> I don't think sending/receiving zero bytes is a good idea in the
> first place.  Which test were failed on MacOS?  Was it failing at
> test-iov "random I/O"?
>
> I thought I ensured that the test does not call any i/o function
> with zero "count" argument.  Might be I was wrong, and in that
> case THAT place should be fixed instead.
>
> Can you provide a bit more details please?

Sure. It fails in test-iov:
#0  0x000000010000103a in do_send_recv (sockfd=4, iov=0x10060ffb0,
iov_cnt=0, do_send=false) at iov.c:101
#1  0x0000000100001318 in iov_send_recv (sockfd=4, iov=0x10060ffb0,
iov_cnt=3, offset=0, bytes=0, do_send=<value temporarily unavailable,
due to optimizations>) at iov.c:179
#2  0x00000001000025c4 in test_io () at tests/test-iov.c:229

because the test does:

      for (i = 0; i <= sz; ++i) {
           for (j = i; j <= sz; ++j) {
               k = i;
               iov_memset(iov, niov, 0, 0xff, -1);
               do {
                   s = g_test_rand_int_range(0, j - k + 1);
                   r = iov_recv(sv[0], iov, niov, k, s);

so on the first time round the loop i=0 so k=0 and we pass
a zero bytes count to iov_recv.

> The whole thing is actually interesting: this is indeed a system-
> dependent corner case which should be handled in the code to make
> the routine consistent.  But how to fix this is an open question
> I think.  Your approach seems to be best, but we as well may
> print a warning there...

Since sendmsg()/recvmsg() do permit you to send nothing by
having a non-zero length vector whose elements are all zero
bytes long, making the zero length vector a no-op seemed to
be the consistent approach.

-- PMM

Re: [Qemu-devel] [PATCH] iov_send_recv(): Handle zero bytes case even if OS does not

[Michael Tokarev]

On 12.08.2012 01:24, Peter Maydell wrote:
> POSIX allows sendmsg() and recvmsg() to fail EMSGSIZE if passed a zero
> msg.msg_iovlen (in particular the MacOS X implementation will do this).
> Handle the case where iov_send_recv() is passed a zero byte count
> explicitly, to avoid accidentally depending on the OS to treat zero
> msg_iovlen as a no-op.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> This is what was causing 'make check' to fail on MacOS X.
> The other option was to declare that a zero bytecount was illegal, I guess.

Acked-by: Michael Tokarev <mjt@tls.msk.ru>

Kevin, does this fix the test-iov failure you're seeing on one of
the build bots?

Thank you!

/mjt

>  iov.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/iov.c b/iov.c
> index b333061..60705c7 100644
> --- a/iov.c
> +++ b/iov.c
> @@ -146,6 +146,13 @@ ssize_t iov_send_recv(int sockfd, struct iovec *iov, unsigned iov_cnt,
>  {
>      ssize_t ret;
>      unsigned si, ei;            /* start and end indexes */
> +    if (bytes == 0) {
> +        /* Catch the do-nothing case early, as otherwise we will pass an
> +         * empty iovec to sendmsg/recvmsg(), and not all implementations
> +         * accept this.
> +         */
> +        return 0;
> +    }
>  
>      /* Find the start position, skipping `offset' bytes:
>       * first, skip all full-sized vector elements, */

Re: [Qemu-devel] [PATCH] iov_send_recv(): Handle zero bytes case even if OS does not

[Kevin Wolf]

Am 12.08.2012 12:32, schrieb Michael Tokarev:
> On 12.08.2012 01:24, Peter Maydell wrote:
>> POSIX allows sendmsg() and recvmsg() to fail EMSGSIZE if passed a zero
>> msg.msg_iovlen (in particular the MacOS X implementation will do this).
>> Handle the case where iov_send_recv() is passed a zero byte count
>> explicitly, to avoid accidentally depending on the OS to treat zero
>> msg_iovlen as a no-op.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>> This is what was causing 'make check' to fail on MacOS X.
>> The other option was to declare that a zero bytecount was illegal, I guess.
> 
> Acked-by: Michael Tokarev <mjt@tls.msk.ru>
> 
> Kevin, does this fix the test-iov failure you're seeing on one of
> the build bots?

It's not on a build bot but on my test machine, but yes, this does fix
it indeed. Thanks for looking into it, Peter!

Kevin

Re: [Qemu-devel] [Qemu-trivial] [PATCH] iov_send_recv(): Handle zero bytes case even if OS does not

[Stefan Hajnoczi]

On Sat, Aug 11, 2012 at 10:24:35PM +0100, Peter Maydell wrote:
> POSIX allows sendmsg() and recvmsg() to fail EMSGSIZE if passed a zero
> msg.msg_iovlen (in particular the MacOS X implementation will do this).
> Handle the case where iov_send_recv() is passed a zero byte count
> explicitly, to avoid accidentally depending on the OS to treat zero
> msg_iovlen as a no-op.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> This is what was causing 'make check' to fail on MacOS X.
> The other option was to declare that a zero bytecount was illegal, I guess.
> 
>  iov.c | 7 +++++++
>  1 file changed, 7 insertions(+)

Thanks, applied to the trivial patches tree:
https://github.com/stefanha/qemu/commits/trivial-patches

Stefan