svn commit: r1806014 - /subversion/trunk/subversion/libsvn_subr/io.c

[ko...@apache.org]

Author: kotkov
Date: Thu Aug 24 09:22:40 2017
New Revision: 1806014

URL: http://svn.apache.org/viewvc?rev=1806014&view=rev
Log:
Fix a potential bug and an API contract violation in the Win32 implementation
of svn_io_file_write_full().

Even in the case when a specific workaround for issue #1789 is not required,
the implementation of this function has been calling apr_file_write() that
can do a short write and return APR_SUCCESS.  If that happens, the
guarantee of the "write full" function would not be fulfilled in the sense
that the function would write less bytes than requested without an
associated error, and that could cause issues further down the line.

On Windows, apr_file_write() is implemented as a call to WriteFile() that
shouldn't result in short writes for file handles, but such short writes
are still possible, for example, for pipes.  That is where the previous
implementation could be working improperly.  (See the "Remarks" section
in https://msdn.microsoft.com/en-us/library/windows/desktop/aa365747).

* subversion/libsvn_subr/io.c
  (svn_io_file_write_full): Always attempt a full write in the Win32-specific
   code path.  Join and tweak the comments on issue #1789, add the issue
   number and place the comment right before the code that implements a
   workaround.

Modified:
    subversion/trunk/subversion/libsvn_subr/io.c

Modified: subversion/trunk/subversion/libsvn_subr/io.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=1806014&r1=1806013&r2=1806014&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/io.c (original)
+++ subversion/trunk/subversion/libsvn_subr/io.c Thu Aug 24 09:22:40 2017
@@ -3940,21 +3940,20 @@ svn_io_file_write_full(apr_file_t *file,
                        apr_size_t nbytes, apr_size_t *bytes_written,
                        apr_pool_t *pool)
 {
-  /* We cannot simply call apr_file_write_full on Win32 as it may fail
-     for larger values of NBYTES. In that case, we have to emulate the
-     "_full" part here. Thus, always call apr_file_write directly on
-     Win32 as this minimizes overhead for small data buffers. */
 #ifdef WIN32
 #define MAXBUFSIZE 30*1024
   apr_size_t bw = nbytes;
   apr_size_t to_write = nbytes;
+  apr_status_t rv;
 
-  /* try a simple "write everything at once" first */
-  apr_status_t rv = apr_file_write(file, buf, &bw);
+  rv = apr_file_write_full(file, buf, nbytes, &bw);
   buf = (char *)buf + bw;
   to_write -= bw;
 
-  /* if the OS cannot handle that, use smaller chunks */
+  /* Issue #1789: On Windows, writing may fail for large values of NBYTES.
+     If that is the case, keep track of how many bytes have been written
+     by the apr_file_write_full() call, and attempt to write the remaining
+     part in smaller chunks. */
   if (rv == APR_FROM_OS_ERROR(ERROR_NOT_ENOUGH_MEMORY)
       && nbytes > MAXBUFSIZE)
     {