[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] tools/libxc: Implement writev_exact() in the same style as write_exact()




On 03/07/2014 18:15, Ian Jackson wrote:
Andrew Cooper writes ("[PATCH] tools/libxc: Implement writev_exact() in the same 
style as write_exact()"):
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.
...
+int writev_exact(int fd, struct iovec *iov, int iovcnt)
+{
+    int iov_idx = 0;
+    ssize_t len;
+
+    while ( iov_idx < iovcnt )
+    {
+        /* Skip over iov[] enties with 0 length. */
+        while ( iov[iov_idx].iov_len == 0 )
+            if ( ++iov_idx == iovcnt )
+                return 0;
+
+        len = writev(fd, &iov[iov_idx], MIN(iovcnt - iov_idx, IOV_MAX));
+
+        if ( (len == -1) && (errno == EINTR) )
+            continue;
+        if ( len <= 0 )
+            return -1;
If writev does return 0, you at the very least need to set errno
before changing the return value to -1.

Hmm - that should probably be "return len", to have the same EOF behaviour as write_exact(). In that case, errno should be correct without further modification.

Errno does however need to be set for the previous return 0.


+        /* Check iov[] to see whether we had a partial or complete write. */
+        while ( len > 0 && (iov_idx < iovcnt) )
+            len -= iov[iov_idx++].iov_len;
+
+        if ( len < 0 ) /* Partial write of iov[iov_idx - 1]. */
+        {
+            iov_idx--;
This logic is rather wtf!  Isn't there a way of expressing it that
doesn't involve len becoming negative and unwinding iov_idx ?

+int writev_exact(int fd, struct iovec *iov, int iovcnt);
+/* Note - writev_exact() might modify iov.  Whether it does so in practice
+ * depends on whether your system implementation of writev() returns from a
+ * partial write in the middle of an iov element. */
The second sentence should be removed.  No-one is allowed to assume
that writev doesn't do so.  Also, you should mention that your
writev_exact lacks the atomicity guarantee of proper writev.

Actually I wonder.

Rethinking this somewhat, the atomicity guarentee of the proper writev() wrt individual iov[] elements constitute a guarentee that it shall never return having performed a partial write of an individual iov[] element. I came to this conclusion first, then somehow argued myself out of it. Can I have a second opinion?

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.