Lets agree there is a bug in the way the function is used in
mod_disk_cache. That will be looked at.
 
As far as not having a bug in the !HAS_WRITEV implementation,
I disagree.
The current implementation does not have a single chance of
being successful if there is more than 1 vector. This does
not make sense to me. Let assume the write part is successful,
the function will return apr_success but has completely failed
to his task.
In the header file there is a remark saying that apr_file_writev
is available even if the underlying os doesn't provide writev().
Why providing something that it is going to fail every single
time?
 
Is the current implementation meets the expectation of others and
I am the only one that see something wrong here?
Thanks,
Jean-Jacques


>>> Joe Orton <jorton@redhat.com> 10/05/04 2:40 PM >>>
On Tue, Oct 05, 2004 at 11:23:35AM -0600, Jean-Jacques Clar wrote:
> >>> <jorton@redhat.com> 10/05/04 12:16 AM >>>
> 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?
>
> If HAS_WRITEV is not defined, there is a bug inside apr_file_writev().
> Only the first vector will be written to the file.
> How do you think that bug should be fixed?

I don't think that's a bug.  apr_file_writev() should be allowed to
return short, as the writev()-based implementation may already do this
too.  So the !HAS_WRITEV implementation is valid.

> apr_file_writev() is used multiple times without alternatives in
> mod_disk_cache.c and the current implementation is failing without
> informing the caller or any failure if writev() is not available.

Right, the bug is in how the function is used.  It's the same as calling
apr_file_write() and presuming it won't return short.  The fix is to
call apr_file_write_full(), in that case.  In this case, there is no
apr_file_writev_full(), so that should be fixed.

joe