apr_file_read() may return an error while bytes were read (windows only)

[Yann Ylavic <yl...@gmail.com>]

whereas the doxygen states :
* @remark It is not possible for both bytes to be read and an #APR_EOF
* or other error to be returned.  #APR_EINTR is never returned.

This may happen when a char is unget, the file is buffered, and the
following flush (file->direction == 1) fails.

Possible fix :
Index: file_io/win32/readwrite.c
===================================================================
--- file_io/win32/readwrite.c    (revision 1601923)
+++ file_io/win32/readwrite.c    (working copy)
@@ -160,6 +160,7 @@ APR_DECLARE(apr_status_t) apr_file_read(apr_file_t
         thefile->pOverlapped->hEvent = CreateEvent(NULL, FALSE, FALSE, NULL);
         if (!thefile->pOverlapped->hEvent) {
             rv = apr_get_os_error();
+            *len = 0;
             return rv;
         }
     }
@@ -191,6 +192,10 @@ APR_DECLARE(apr_status_t) apr_file_read(apr_file_t
                 if (thefile->flags & APR_FOPEN_XTHREAD) {
                     apr_thread_mutex_unlock(thefile->mutex);
                 }
+                if (bytes_read) {
+                    *len = bytes_read;
+                    return APR_SUCCESS;
+                }
                 return rv;
             }
             thefile->bufpos = 0;
[END]

The first patch chunk ensures that *len is 0 when this error occurs
(no data are read yet), since the @remark above also implies that
(*len > 0) => APR_SUCCESS.

Regards,
Yann.

Re: apr_file_read() may return an error while bytes were read (windows only)

[Yann Ylavic <yl...@gmail.com>]

On Thu, Jun 12, 2014 at 4:12 PM, Yann Ylavic <yl...@gmail.com> wrote:
> @@ -191,6 +192,10 @@ APR_DECLARE(apr_status_t) apr_file_read(apr_file_t
>                  if (thefile->flags & APR_FOPEN_XTHREAD) {
>                      apr_thread_mutex_unlock(thefile->mutex);
>                  }
> +                if (bytes_read) {
> +                    *len = bytes_read;
> +                    return APR_SUCCESS;
> +                }

Hm, I guess the following is better (forces *len = 0 on error with no data) :
+                *len = bytes_read;
+                if (*len) {
+                    return APR_SUCCESS;
+                }

>                  return rv;