[Patch v3 1/2] tools/libxc: Shuffle definitions and uses of min()/max() macros

[Patch v3 1/2] tools/libxc: Shuffle definitions and uses of min()/max() macros

[Andrew Cooper]

Move the typesafe min() macro from xc_dom_decompress_unsafe_xz.c to
xc_private.h, and complement it with an equivalent max() macro.

Replace current users of type unsafe MIN()/MAX() macros, and remove their
scattered definitions.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>

---
v3: Rebase on top of min_t()/max_t() changes.
---
 tools/libxc/xc_core_x86.c                 |    4 ----
 tools/libxc/xc_dom_decompress_unsafe_xz.c |    6 ------
 tools/libxc/xc_domain_restore.c           |    2 +-
 tools/libxc/xc_private.h                  |   11 +++++++++++
 tools/libxc/xg_private.h                  |    2 +-
 tools/libxc/xg_save_restore.h             |    7 -------
 6 files changed, 13 insertions(+), 19 deletions(-)

diff --git a/tools/libxc/xc_core_x86.c b/tools/libxc/xc_core_x86.c
index e328dcf..f05060a 100644
--- a/tools/libxc/xc_core_x86.c
+++ b/tools/libxc/xc_core_x86.c
@@ -24,10 +24,6 @@
 
 #define GET_FIELD(_p, _f) ((dinfo->guest_width==8) ? ((_p)->x64._f) : ((_p)->x32._f))
 
-#ifndef MAX
-#define MAX(_a, _b) ((_a) >= (_b) ? (_a) : (_b))
-#endif
-
 int
 xc_core_arch_gpfn_may_present(struct xc_core_arch_context *arch_ctxt,
                               unsigned long pfn)
diff --git a/tools/libxc/xc_dom_decompress_unsafe_xz.c b/tools/libxc/xc_dom_decompress_unsafe_xz.c
index 6be1f89..fe7a7f4 100644
--- a/tools/libxc/xc_dom_decompress_unsafe_xz.c
+++ b/tools/libxc/xc_dom_decompress_unsafe_xz.c
@@ -34,12 +34,6 @@ static inline u32 le32_to_cpup(const u32 *p)
 	return cpu_to_le32(*p);
 }
 
-#define min(x,y) ({ \
-        const typeof(x) _x = (x);       \
-        const typeof(y) _y = (y);       \
-        (void) (&_x == &_y);            \
-        _x < _y ? _x : _y; })
-
 #define __force
 #define always_inline
 
diff --git a/tools/libxc/xc_domain_restore.c b/tools/libxc/xc_domain_restore.c
index 071ab6a..e73e0a2 100644
--- a/tools/libxc/xc_domain_restore.c
+++ b/tools/libxc/xc_domain_restore.c
@@ -339,7 +339,7 @@ static xen_pfn_t *load_p2m_frame_list(
             /* Any remaining bytes of this chunk: read and discard. */
             while ( chunk_bytes )
             {
-                unsigned long sz = MIN(chunk_bytes, sizeof(xen_pfn_t));
+                unsigned long sz = min_t(unsigned long, chunk_bytes, sizeof(xen_pfn_t));
                 if ( RDEXACT(io_fd, &p2m_fl_zero, sz) )
                 {
                     PERROR("read-and-discard extended-info chunk bytes failed");
diff --git a/tools/libxc/xc_private.h b/tools/libxc/xc_private.h
index c240a7c..ce7e81e 100644
--- a/tools/libxc/xc_private.h
+++ b/tools/libxc/xc_private.h
@@ -349,6 +349,17 @@ int xc_ffs16(uint16_t x);
 int xc_ffs32(uint32_t x);
 int xc_ffs64(uint64_t x);
 
+#define min(X, Y) ({                             \
+            const typeof (X) _x = (X);           \
+            const typeof (Y) _y = (Y);           \
+            (void) (&_x == &_y);                 \
+            (_x < _y) ? _x : _y; })
+#define max(X, Y) ({                             \
+            const typeof (X) _x = (X);           \
+            const typeof (Y) _y = (Y);           \
+            (void) (&_x == &_y);                 \
+            (_x > _y) ? _x : _y; })
+
 #define min_t(type,x,y) \
         ({ type __x = (x); type __y = (y); __x < __y ? __x: __y; })
 #define max_t(type,x,y) \
diff --git a/tools/libxc/xg_private.h b/tools/libxc/xg_private.h
index e593364..1910361 100644
--- a/tools/libxc/xg_private.h
+++ b/tools/libxc/xg_private.h
@@ -159,7 +159,7 @@ static inline xen_pfn_t xc_pfn_to_mfn(xen_pfn_t pfn, xen_pfn_t *p2m,
 /* Size in bytes of the pfn_to_mfn_frame_list     */
 #define P2M_GUEST_FL_SIZE ((P2M_FL_ENTRIES) * (dinfo->guest_width))
 #define P2M_TOOLS_FL_SIZE ((P2M_FL_ENTRIES) *                           \
-                           MAX((sizeof (xen_pfn_t)), dinfo->guest_width))
+                           max_t(size_t, sizeof(xen_pfn_t), dinfo->guest_width))
 
 /* Masks for PTE<->PFN conversions */
 #define MADDR_BITS_X86  ((dinfo->guest_width == 8) ? 52 : 44)
diff --git a/tools/libxc/xg_save_restore.h b/tools/libxc/xg_save_restore.h
index aa93c13..bdd9009 100644
--- a/tools/libxc/xg_save_restore.h
+++ b/tools/libxc/xg_save_restore.h
@@ -392,10 +392,3 @@ static inline int get_platform_info(xc_interface *xch, uint32_t dom,
     else                                                           \
         memset(&(_p)->x32._f[0], (_v), sizeof((_p)->x32._f));      \
 } while (0)
-
-#ifndef MAX
-#define MAX(_a, _b) ((_a) >= (_b) ? (_a) : (_b))
-#endif
-#ifndef MIN
-#define MIN(_a, _b) ((_a) <= (_b) ? (_a) : (_b))
-#endif
-- 
1.7.10.4

[Patch v3 2/2] tools/libxc: Implement writev_exact() in the same style as write_exact()

[Andrew Cooper]

This implementation of writev_exact() will cope with an iovcnt greater than
IOV_MAX because glibc will actually let this work anyway, and it is very
useful not to have to work about this in the caller of writev_exact().  The
caller is still required to ensure that the sum of iov_len's doesn't overflow
a ssize_t.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>

---
v3:
 * Re-add adjustment for partial writes.
 * Split min/max adjustment into separate patch.

v2:
 * Remove adjustment for partial writes of a specific iov[] entry.
---
 tools/libxc/xc_private.c |   60 ++++++++++++++++++++++++++++++++++++++++++++++
 tools/libxc/xc_private.h |    2 ++
 2 files changed, 62 insertions(+)

diff --git a/tools/libxc/xc_private.c b/tools/libxc/xc_private.c
index 1c214dd..0941b06 100644
--- a/tools/libxc/xc_private.c
+++ b/tools/libxc/xc_private.c
@@ -858,6 +858,66 @@ int write_exact(int fd, const void *data, size_t size)
     return 0;
 }
 
+int writev_exact(int fd, const struct iovec *iov, int iovcnt)
+{
+    struct iovec *local_iov = NULL;
+    int rc = 0, iov_idx = 0, saved_errno = 0;
+    ssize_t len;
+
+    while ( iov_idx < iovcnt )
+    {
+        /* Skip over iov[] entries with 0 length. */
+        while ( iov[iov_idx].iov_len == 0 )
+            if ( ++iov_idx == iovcnt )
+                goto out;
+
+        len = writev(fd, &iov[iov_idx], min(iovcnt - iov_idx, IOV_MAX));
+        saved_errno = errno;
+
+        if ( (len == -1) && (errno == EINTR) )
+            continue;
+        if ( len <= 0 )
+        {
+            rc = -1;
+            goto out;
+        }
+
+        /* Check iov[] to see whether we had a partial or complete write. */
+        while ( len > 0 && (iov_idx < iovcnt) )
+        {
+            if ( len >= iov[iov_idx].iov_len )
+                len -= iov[iov_idx++].iov_len;
+            else
+            {
+                /* Partial write of iov[iov_idx]. Copy iov so we can adjust
+                 * element iov_idx and resubmit the rest. */
+                if ( !local_iov )
+                {
+                    local_iov = malloc(iovcnt * sizeof(*iov));
+                    if ( !local_iov )
+                    {
+                        saved_errno = ENOMEM;
+                        goto out;
+                    }
+
+                    iov = memcpy(local_iov, iov, iovcnt * sizeof(*iov));
+                }
+
+                local_iov[iov_idx].iov_base += len;
+                local_iov[iov_idx].iov_len  -= len;
+                break;
+            }
+        }
+    }
+
+    saved_errno = 0;
+
+ out:
+    free(local_iov);
+    errno = saved_errno;
+    return rc;
+}
+
 int xc_ffs8(uint8_t x)
 {
     int i;
diff --git a/tools/libxc/xc_private.h b/tools/libxc/xc_private.h
index ce7e81e..1f06166 100644
--- a/tools/libxc/xc_private.h
+++ b/tools/libxc/xc_private.h
@@ -28,6 +28,7 @@
 #include <sys/stat.h>
 #include <stdlib.h>
 #include <sys/ioctl.h>
+#include <sys/uio.h>
 
 #include "xenctrl.h"
 #include "xenctrlosdep.h"
@@ -343,6 +344,7 @@ int xc_flush_mmu_updates(xc_interface *xch, struct xc_mmu *mmu);
 /* Return 0 on success; -1 on error setting errno. */
 int read_exact(int fd, void *data, size_t size); /* EOF => -1, errno=0 */
 int write_exact(int fd, const void *data, size_t size);
+int writev_exact(int fd, const struct iovec *iov, int iovcnt);
 
 int xc_ffs8(uint8_t x);
 int xc_ffs16(uint16_t x);
-- 
1.7.10.4

Re: [Patch v3 1/2] tools/libxc: Shuffle definitions and uses of min()/max() macros

[Ian Campbell]

On Wed, 2014-07-16 at 15:32 +0100, Andrew Cooper wrote:
> Move the typesafe min() macro from xc_dom_decompress_unsafe_xz.c to
> xc_private.h, and complement it with an equivalent max() macro.
> 
> Replace current users of type unsafe MIN()/MAX() macros, and remove their
> scattered definitions.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Ian Campbell <Ian.Campbell@citrix.com>

Re: [Patch v3 2/2] tools/libxc: Implement writev_exact() in the same style as write_exact()

[Wen Congyang]

At 07/16/2014 10:32 PM, Andrew Cooper Wrote:
> This implementation of writev_exact() will cope with an iovcnt greater than
> IOV_MAX because glibc will actually let this work anyway, and it is very
> useful not to have to work about this in the caller of writev_exact().  The
> caller is still required to ensure that the sum of iov_len's doesn't overflow
> a ssize_t.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Ian Campbell <Ian.Campbell@citrix.com>
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> 
> ---
> v3:
>  * Re-add adjustment for partial writes.
>  * Split min/max adjustment into separate patch.
> 
> v2:
>  * Remove adjustment for partial writes of a specific iov[] entry.
> ---
>  tools/libxc/xc_private.c |   60 ++++++++++++++++++++++++++++++++++++++++++++++
>  tools/libxc/xc_private.h |    2 ++
>  2 files changed, 62 insertions(+)
> 
> diff --git a/tools/libxc/xc_private.c b/tools/libxc/xc_private.c
> index 1c214dd..0941b06 100644
> --- a/tools/libxc/xc_private.c
> +++ b/tools/libxc/xc_private.c
> @@ -858,6 +858,66 @@ int write_exact(int fd, const void *data, size_t size)
>      return 0;
>  }
>  
> +int writev_exact(int fd, const struct iovec *iov, int iovcnt)
> +{
> +    struct iovec *local_iov = NULL;
> +    int rc = 0, iov_idx = 0, saved_errno = 0;
> +    ssize_t len;
> +
> +    while ( iov_idx < iovcnt )
> +    {
> +        /* Skip over iov[] entries with 0 length. */
> +        while ( iov[iov_idx].iov_len == 0 )
> +            if ( ++iov_idx == iovcnt )
> +                goto out;

set saved_errn to 0 before goto out?

> +
> +        len = writev(fd, &iov[iov_idx], min(iovcnt - iov_idx, IOV_MAX));
> +        saved_errno = errno;
> +
> +        if ( (len == -1) && (errno == EINTR) )
> +            continue;
> +        if ( len <= 0 )
> +        {
> +            rc = -1;
> +            goto out;
> +        }
> +
> +        /* Check iov[] to see whether we had a partial or complete write. */
> +        while ( len > 0 && (iov_idx < iovcnt) )
> +        {
> +            if ( len >= iov[iov_idx].iov_len )
> +                len -= iov[iov_idx++].iov_len;
> +            else
> +            {
> +                /* Partial write of iov[iov_idx]. Copy iov so we can adjust
> +                 * element iov_idx and resubmit the rest. */
> +                if ( !local_iov )
> +                {
> +                    local_iov = malloc(iovcnt * sizeof(*iov));
> +                    if ( !local_iov )
> +                    {
> +                        saved_errno = ENOMEM;
> +                        goto out;
> +                    }
> +
> +                    iov = memcpy(local_iov, iov, iovcnt * sizeof(*iov));
> +                }
> +
> +                local_iov[iov_idx].iov_base += len;
> +                local_iov[iov_idx].iov_len  -= len;
> +                break;
> +            }
> +        }
> +    }
> +
> +    saved_errno = 0;
> +
> + out:
> +    free(local_iov);
> +    errno = saved_errno;
> +    return rc;
> +}
> +
>  int xc_ffs8(uint8_t x)
>  {
>      int i;
> diff --git a/tools/libxc/xc_private.h b/tools/libxc/xc_private.h
> index ce7e81e..1f06166 100644
> --- a/tools/libxc/xc_private.h
> +++ b/tools/libxc/xc_private.h
> @@ -28,6 +28,7 @@
>  #include <sys/stat.h>
>  #include <stdlib.h>
>  #include <sys/ioctl.h>
> +#include <sys/uio.h>
>  
>  #include "xenctrl.h"
>  #include "xenctrlosdep.h"
> @@ -343,6 +344,7 @@ int xc_flush_mmu_updates(xc_interface *xch, struct xc_mmu *mmu);
>  /* Return 0 on success; -1 on error setting errno. */
>  int read_exact(int fd, void *data, size_t size); /* EOF => -1, errno=0 */
>  int write_exact(int fd, const void *data, size_t size);
> +int writev_exact(int fd, const struct iovec *iov, int iovcnt);
>  
>  int xc_ffs8(uint8_t x);
>  int xc_ffs16(uint16_t x);
> 

Re: [Patch v3 2/2] tools/libxc: Implement writev_exact() in the same style as write_exact()

[Andrew Cooper]

On 18/07/14 02:14, Wen Congyang wrote:
> At 07/16/2014 10:32 PM, Andrew Cooper Wrote:
>> This implementation of writev_exact() will cope with an iovcnt greater than
>> IOV_MAX because glibc will actually let this work anyway, and it is very
>> useful not to have to work about this in the caller of writev_exact().  The
>> caller is still required to ensure that the sum of iov_len's doesn't overflow
>> a ssize_t.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> CC: Ian Campbell <Ian.Campbell@citrix.com>
>> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
>>
>> ---
>> v3:
>>  * Re-add adjustment for partial writes.
>>  * Split min/max adjustment into separate patch.
>>
>> v2:
>>  * Remove adjustment for partial writes of a specific iov[] entry.
>> ---
>>  tools/libxc/xc_private.c |   60 ++++++++++++++++++++++++++++++++++++++++++++++
>>  tools/libxc/xc_private.h |    2 ++
>>  2 files changed, 62 insertions(+)
>>
>> diff --git a/tools/libxc/xc_private.c b/tools/libxc/xc_private.c
>> index 1c214dd..0941b06 100644
>> --- a/tools/libxc/xc_private.c
>> +++ b/tools/libxc/xc_private.c
>> @@ -858,6 +858,66 @@ int write_exact(int fd, const void *data, size_t size)
>>      return 0;
>>  }
>>  
>> +int writev_exact(int fd, const struct iovec *iov, int iovcnt)
>> +{
>> +    struct iovec *local_iov = NULL;
>> +    int rc = 0, iov_idx = 0, saved_errno = 0;
>> +    ssize_t len;
>> +
>> +    while ( iov_idx < iovcnt )
>> +    {
>> +        /* Skip over iov[] entries with 0 length. */
>> +        while ( iov[iov_idx].iov_len == 0 )
>> +            if ( ++iov_idx == iovcnt )
>> +                goto out;
> set saved_errn to 0 before goto out?

Good catch.

~Andrew

[PATCH v4 2/2] tools/libxc: Implement writev_exact() in the same style as write_exact()

[Andrew Cooper]

This implementation of writev_exact() will cope with an iovcnt greater than
IOV_MAX because glibc will actually let this work anyway, and it is very
useful not to have to work about this in the caller of writev_exact().  The
caller is still required to ensure that the sum of iov_len's doesn't overflow
a ssize_t.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>

---
v4:
 * Set errno to 0 when exiting due to 0-length iov[]s.
v3:
 * Re-add adjustment for partial writes.
 * Split min/max adjustment into separate patch.
v2:
 * Remove adjustment for partial writes of a specific iov[] entry.
---
 tools/libxc/xc_private.c |   63 ++++++++++++++++++++++++++++++++++++++++++++++
 tools/libxc/xc_private.h |    2 ++
 2 files changed, 65 insertions(+)

diff --git a/tools/libxc/xc_private.c b/tools/libxc/xc_private.c
index 1c214dd..338dc5a 100644
--- a/tools/libxc/xc_private.c
+++ b/tools/libxc/xc_private.c
@@ -858,6 +858,69 @@ int write_exact(int fd, const void *data, size_t size)
     return 0;
 }
 
+int writev_exact(int fd, const struct iovec *iov, int iovcnt)
+{
+    struct iovec *local_iov = NULL;
+    int rc = 0, iov_idx = 0, saved_errno = 0;
+    ssize_t len;
+
+    while ( iov_idx < iovcnt )
+    {
+        /* Skip over iov[] entries with 0 length. */
+        while ( iov[iov_idx].iov_len == 0 )
+            if ( ++iov_idx == iovcnt )
+            {
+                saved_errno = 0;
+                goto out;
+            }
+
+        len = writev(fd, &iov[iov_idx], min(iovcnt - iov_idx, IOV_MAX));
+        saved_errno = errno;
+
+        if ( (len == -1) && (errno == EINTR) )
+            continue;
+        if ( len <= 0 )
+        {
+            rc = -1;
+            goto out;
+        }
+
+        /* Check iov[] to see whether we had a partial or complete write. */
+        while ( len > 0 && (iov_idx < iovcnt) )
+        {
+            if ( len >= iov[iov_idx].iov_len )
+                len -= iov[iov_idx++].iov_len;
+            else
+            {
+                /* Partial write of iov[iov_idx]. Copy iov so we can adjust
+                 * element iov_idx and resubmit the rest. */
+                if ( !local_iov )
+                {
+                    local_iov = malloc(iovcnt * sizeof(*iov));
+                    if ( !local_iov )
+                    {
+                        saved_errno = ENOMEM;
+                        goto out;
+                    }
+
+                    iov = memcpy(local_iov, iov, iovcnt * sizeof(*iov));
+                }
+
+                local_iov[iov_idx].iov_base += len;
+                local_iov[iov_idx].iov_len  -= len;
+                break;
+            }
+        }
+    }
+
+    saved_errno = 0;
+
+ out:
+    free(local_iov);
+    errno = saved_errno;
+    return rc;
+}
+
 int xc_ffs8(uint8_t x)
 {
     int i;
diff --git a/tools/libxc/xc_private.h b/tools/libxc/xc_private.h
index ce7e81e..1f06166 100644
--- a/tools/libxc/xc_private.h
+++ b/tools/libxc/xc_private.h
@@ -28,6 +28,7 @@
 #include <sys/stat.h>
 #include <stdlib.h>
 #include <sys/ioctl.h>
+#include <sys/uio.h>
 
 #include "xenctrl.h"
 #include "xenctrlosdep.h"
@@ -343,6 +344,7 @@ int xc_flush_mmu_updates(xc_interface *xch, struct xc_mmu *mmu);
 /* Return 0 on success; -1 on error setting errno. */
 int read_exact(int fd, void *data, size_t size); /* EOF => -1, errno=0 */
 int write_exact(int fd, const void *data, size_t size);
+int writev_exact(int fd, const struct iovec *iov, int iovcnt);
 
 int xc_ffs8(uint8_t x);
 int xc_ffs16(uint16_t x);
-- 
1.7.10.4

Re: [Patch v3 2/2] tools/libxc: Implement writev_exact() in the same style as write_exact()

[Ian Campbell]

On Fri, 2014-07-18 at 10:20 +0100, Andrew Cooper wrote:
> On 18/07/14 02:14, Wen Congyang wrote:
> > At 07/16/2014 10:32 PM, Andrew Cooper Wrote:
> >> This implementation of writev_exact() will cope with an iovcnt greater than
> >> IOV_MAX because glibc will actually let this work anyway, and it is very
> >> useful not to have to work about this in the caller of writev_exact().  The
> >> caller is still required to ensure that the sum of iov_len's doesn't overflow
> >> a ssize_t.
> >>
> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> >> CC: Ian Campbell <Ian.Campbell@citrix.com>
> >> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> >>
> >> ---
> >> v3:
> >>  * Re-add adjustment for partial writes.
> >>  * Split min/max adjustment into separate patch.
> >>
> >> v2:
> >>  * Remove adjustment for partial writes of a specific iov[] entry.
> >> ---
> >>  tools/libxc/xc_private.c |   60 ++++++++++++++++++++++++++++++++++++++++++++++
> >>  tools/libxc/xc_private.h |    2 ++
> >>  2 files changed, 62 insertions(+)
> >>
> >> diff --git a/tools/libxc/xc_private.c b/tools/libxc/xc_private.c
> >> index 1c214dd..0941b06 100644
> >> --- a/tools/libxc/xc_private.c
> >> +++ b/tools/libxc/xc_private.c
> >> @@ -858,6 +858,66 @@ int write_exact(int fd, const void *data, size_t size)
> >>      return 0;
> >>  }
> >>  
> >> +int writev_exact(int fd, const struct iovec *iov, int iovcnt)
> >> +{
> >> +    struct iovec *local_iov = NULL;
> >> +    int rc = 0, iov_idx = 0, saved_errno = 0;
> >> +    ssize_t len;
> >> +
> >> +    while ( iov_idx < iovcnt )
> >> +    {
> >> +        /* Skip over iov[] entries with 0 length. */
> >> +        while ( iov[iov_idx].iov_len == 0 )
> >> +            if ( ++iov_idx == iovcnt )
> >> +                goto out;
> > set saved_errn to 0 before goto out?
> 
> Good catch.

Isn't this a success path? errno is generally undefined on success.

Ian.

Re: [Patch v3 2/2] tools/libxc: Implement writev_exact() in the same style as write_exact()

[Wen Congyang]

At 07/18/2014 05:53 PM, Ian Campbell Wrote:
> On Fri, 2014-07-18 at 10:20 +0100, Andrew Cooper wrote:
>> On 18/07/14 02:14, Wen Congyang wrote:
>>> At 07/16/2014 10:32 PM, Andrew Cooper Wrote:
>>>> This implementation of writev_exact() will cope with an iovcnt greater than
>>>> IOV_MAX because glibc will actually let this work anyway, and it is very
>>>> useful not to have to work about this in the caller of writev_exact().  The
>>>> caller is still required to ensure that the sum of iov_len's doesn't overflow
>>>> a ssize_t.
>>>>
>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>> CC: Ian Campbell <Ian.Campbell@citrix.com>
>>>> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
>>>>
>>>> ---
>>>> v3:
>>>>  * Re-add adjustment for partial writes.
>>>>  * Split min/max adjustment into separate patch.
>>>>
>>>> v2:
>>>>  * Remove adjustment for partial writes of a specific iov[] entry.
>>>> ---
>>>>  tools/libxc/xc_private.c |   60 ++++++++++++++++++++++++++++++++++++++++++++++
>>>>  tools/libxc/xc_private.h |    2 ++
>>>>  2 files changed, 62 insertions(+)
>>>>
>>>> diff --git a/tools/libxc/xc_private.c b/tools/libxc/xc_private.c
>>>> index 1c214dd..0941b06 100644
>>>> --- a/tools/libxc/xc_private.c
>>>> +++ b/tools/libxc/xc_private.c
>>>> @@ -858,6 +858,66 @@ int write_exact(int fd, const void *data, size_t size)
>>>>      return 0;
>>>>  }
>>>>  
>>>> +int writev_exact(int fd, const struct iovec *iov, int iovcnt)
>>>> +{
>>>> +    struct iovec *local_iov = NULL;
>>>> +    int rc = 0, iov_idx = 0, saved_errno = 0;
>>>> +    ssize_t len;
>>>> +
>>>> +    while ( iov_idx < iovcnt )
>>>> +    {
>>>> +        /* Skip over iov[] entries with 0 length. */
>>>> +        while ( iov[iov_idx].iov_len == 0 )
>>>> +            if ( ++iov_idx == iovcnt )
>>>> +                goto out;
>>> set saved_errn to 0 before goto out?
>>
>> Good catch.
> 
> Isn't this a success path? errno is generally undefined on success.

Yes, but we set saved_errno to 0 here:
> +    saved_errno = 0;
> +
> + out:
> +    free(local_iov);
> +    errno = saved_errno;
> +    return rc;
> +}

I think there is no need to save errno in this function, because
we return -1 when writev()/malloc() fails.

Another problem:
> +                    local_iov = malloc(iovcnt * sizeof(*iov));
> +                    if ( !local_iov )
> +                    {
> +                        saved_errno = ENOMEM;
> +                        goto out;
> +                    }
> +
rc is not set to -1 before goto out.

Thanks
Wen Congyang

> 
> Ian.
> 
> .
> 

Re: [Patch v3 2/2] tools/libxc: Implement writev_exact() in the same style as write_exact()

[Andrew Cooper]

On 18/07/14 11:05, Wen Congyang wrote:
> At 07/18/2014 05:53 PM, Ian Campbell Wrote:
>> On Fri, 2014-07-18 at 10:20 +0100, Andrew Cooper wrote:
>>> On 18/07/14 02:14, Wen Congyang wrote:
>>>> At 07/16/2014 10:32 PM, Andrew Cooper Wrote:
>>>>> This implementation of writev_exact() will cope with an iovcnt greater than
>>>>> IOV_MAX because glibc will actually let this work anyway, and it is very
>>>>> useful not to have to work about this in the caller of writev_exact().  The
>>>>> caller is still required to ensure that the sum of iov_len's doesn't overflow
>>>>> a ssize_t.
>>>>>
>>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>>> CC: Ian Campbell <Ian.Campbell@citrix.com>
>>>>> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
>>>>>
>>>>> ---
>>>>> v3:
>>>>>  * Re-add adjustment for partial writes.
>>>>>  * Split min/max adjustment into separate patch.
>>>>>
>>>>> v2:
>>>>>  * Remove adjustment for partial writes of a specific iov[] entry.
>>>>> ---
>>>>>  tools/libxc/xc_private.c |   60 ++++++++++++++++++++++++++++++++++++++++++++++
>>>>>  tools/libxc/xc_private.h |    2 ++
>>>>>  2 files changed, 62 insertions(+)
>>>>>
>>>>> diff --git a/tools/libxc/xc_private.c b/tools/libxc/xc_private.c
>>>>> index 1c214dd..0941b06 100644
>>>>> --- a/tools/libxc/xc_private.c
>>>>> +++ b/tools/libxc/xc_private.c
>>>>> @@ -858,6 +858,66 @@ int write_exact(int fd, const void *data, size_t size)
>>>>>      return 0;
>>>>>  }
>>>>>  
>>>>> +int writev_exact(int fd, const struct iovec *iov, int iovcnt)
>>>>> +{
>>>>> +    struct iovec *local_iov = NULL;
>>>>> +    int rc = 0, iov_idx = 0, saved_errno = 0;
>>>>> +    ssize_t len;
>>>>> +
>>>>> +    while ( iov_idx < iovcnt )
>>>>> +    {
>>>>> +        /* Skip over iov[] entries with 0 length. */
>>>>> +        while ( iov[iov_idx].iov_len == 0 )
>>>>> +            if ( ++iov_idx == iovcnt )
>>>>> +                goto out;
>>>> set saved_errn to 0 before goto out?
>>> Good catch.
>> Isn't this a success path? errno is generally undefined on success.
> Yes, but we set saved_errno to 0 here:
>> +    saved_errno = 0;
>> +
>> + out:
>> +    free(local_iov);
>> +    errno = saved_errno;
>> +    return rc;
>> +}
> I think there is no need to save errno in this function, because
> we return -1 when writev()/malloc() fails.
>
> Another problem:

I am deliberately creating the same (somewhat quirky) error semantics of
write_exact(), for consistency reasons.

The key point is that EOF is -1 and errno of 0.

>> +                    local_iov = malloc(iovcnt * sizeof(*iov));
>> +                    if ( !local_iov )
>> +                    {
>> +                        saved_errno = ENOMEM;
>> +                        goto out;
>> +                    }
>> +
> rc is not set to -1 before goto out.
>
> Thanks
> Wen Congyang

Good point.

~Andrew

Re: [Patch v3 1/2] tools/libxc: Shuffle definitions and uses of min()/max() macros

[Ian Campbell]

On Thu, 2014-07-17 at 15:15 +0100, Ian Campbell wrote:
> On Wed, 2014-07-16 at 15:32 +0100, Andrew Cooper wrote:
> > Move the typesafe min() macro from xc_dom_decompress_unsafe_xz.c to
> > xc_private.h, and complement it with an equivalent max() macro.
> > 
> > Replace current users of type unsafe MIN()/MAX() macros, and remove their
> > scattered definitions.
> > 
> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> Acked-by: Ian Campbell <Ian.Campbell@citrix.com>

Applied, thanks.