[PATCH] apr_file_writev() on UNIX

[Jean-Jacques Clar <JJ...@novell.com>]

If HAS_WRITEV is not defined the current code
will just push the first vector to the target.
The function should write all the vectors or none.
I propose the following patch, comments will be
appreciated:
Thanks for looking.
JJ
 
Index: srclib/apr/file_io/unix/readwrite.c
===================================================================
RCS file: /home/cvs/apr/file_io/unix/readwrite.c,v
retrieving revision 1.90
diff -u -r1.90 readwrite.c
--- srclib/apr/file_io/unix/readwrite.c 2 Aug 2004 09:47:48 -0000 1.90
+++ srclib/apr/file_io/unix/readwrite.c 4 Oct 2004 22:41:45 -0000
@@ -241,8 +241,22 @@
         return APR_SUCCESS;
     }
 #else
-    *nbytes = vec[0].iov_len;
-    return apr_file_write(thefile, vec[0].iov_base, nbytes);
+    apr_status_t rv;
+    int i, bytes = 0;
+
+    for (i = 0; i < nvec; i++) {
+        *nbytes = vec[i].iov_len;
+        if ((rv = apr_file_write(thefile, vec[i].iov_base, nbytes)) !=
APR_SUCCESS) {
+            *nbytes = bytes;
+            return rv;
+        }
+        else {
+            bytes += *nbytes;
+        }
+    }
+    *nbytes = bytes;
+    
+    return APR_SUCCESS;
 #endif
 }

 

Re: [PATCH] apr_file_writev() on UNIX

[jo...@redhat.com]

On Mon, Oct 04, 2004 at 06:11:57PM -0600, Jean-Jacques Clar wrote:
> If HAS_WRITEV is not defined the current code
> will just push the first vector to the target.
> The function should write all the vectors or none.

There is no guarantee that writev(2) writes all the vectors, it's
allowed to return short just like write(2) is, so why should
apr_file_writev() give such a guarantee only for the non-writev-based
implementation?

What you're looking for, perhaps, is an apr_file_writev_full(), which
guarantees "all bytes are written, or failure"?

joe

Re: [PATCH] apr_file_writev() on UNIX

[Cliff Woolley <jw...@virginia.edu>]

On Mon, 4 Oct 2004, Jean-Jacques Clar wrote:

> If HAS_WRITEV is not defined the current code
> will just push the first vector to the target.

Oooh, that sucks.  Good catch.


> The function should write all the vectors or none.
> I propose the following patch, comments will be

+1, with the following nit:


> +    apr_status_t rv;
> +    int i, bytes = 0;
> +
> +    for (i = 0; i < nvec; i++) {
> +        *nbytes = vec[i].iov_len;

Is there really a point to the above line?  *nbytes will always be
overwritten before returning anyway, so why not just skip this line and
then:

> +        if ((rv = apr_file_write(thefile, vec[i].iov_base, nbytes)) !=
> APR_SUCCESS) {
> +            *nbytes = bytes;
> +            return rv;
> +        }
> +        else {
> +            bytes += *nbytes;

bytes += vec[i].iov_len;

> +        }
> +    }
> +    *nbytes = bytes;
> +
> +    return APR_SUCCESS;

Thoughts?

--Cliff