update signature of ReadFile() to match underlying API

This commit updates the signature of ReadFile() to
better match the underlying Windows API. By doing so
we should be able to better support overlapped I/O and
give the user of this function greater control over the
input and output.

For a complete discussion around why this change was made
see #141.

docs/source/changelog.rst

@@ -15,8 +15,8 @@ Notable enhancements and changes are:
 
     * :issue:`141` - :func:`pywincffi.kernel32.file.ReadFile` has been updated
       to provide improved support for overlapped IO. This is a **breaking change**
-      because the type of the returned value has changed and requires additional
-      processing.
+      because the return value and signature of the ReadFile function has
+      changed.
 
 0.5.0
 ~~~~~

pywincffi/kernel32/file.py

@@ -9,7 +9,7 @@
 
 from pywincffi.core import dist
 from pywincffi.core.checks import NON_ZERO, input_check, error_check, NoneType
-from pywincffi.exceptions import WindowsAPIError
+from pywincffi.exceptions import WindowsAPIError, InputError
 from pywincffi.wintypes import (
     SECURITY_ATTRIBUTES, OVERLAPPED, HANDLE, wintype_to_cdata
 )
@@ -193,9 +193,10 @@ def FlushFileBuffers(hFile):
     error_check("FlushFileBuffers", code=code, expected=NON_ZERO)
 
 
-def ReadFile(hFile, nNumberOfBytesToRead, lpOverlapped=None):
+# TODO: Currently lpBuffer only supports bytearray. Other types could be?
+def ReadFile(hFile, lpBuffer, nNumberOfBytesToRead, lpOverlapped=None):
     """
-    Read the specified number of bytes from ``hFile``.
+    Reads data from the specified file or I/O device.
 
     .. seealso::
 
@@ -204,10 +205,18 @@ def ReadFile(hFile, nNumberOfBytesToRead, lpOverlapped=None):
     :param pywincffi.wintypes.HANDLE hFile:
         The handle to read from.
 
+    :param bytearray lpBuffer:
+        The array to receive the data that was read.
+
+        .. note::
+
+            The provided ``lpBuffer`` must be at least as large
+            as ``nNumberOfBytesToRead``.
+
     :param int nNumberOfBytesToRead:
-        The number of bytes to read from ``hFile``
+        The maximum number of bytes to be read.
 
-    :keyword pywincffi.wintypes.OVERLAPPED lpOverlapped:
+    :keyword OVERLAPPED lpOverlapped:
         See Microsoft's documentation for intended usage and below for
         an example.
 
@@ -220,31 +229,38 @@ def ReadFile(hFile, nNumberOfBytesToRead, lpOverlapped=None):
         >>> read_data = ReadFile(  # read 12 bytes from hFile
         ...     hFile, 12, lpOverlapped=lpOverlapped)
 
+    :raises InputError:
+        In addition to being raised for type issues :class:`InputError` will
+        also be raised if the size of the provided ``lpBuffer`` is smaller
+        than ``nNumberOfBytesToRead``.
+
     :returns:
-        Returns a Python bytearray. If the input ``hFile`` was not
-        opened in overlapped mode then the returned array will contain
-        the resulting data. If ``hFile`` was opened in overlapped mode
-        then the read data will be pushed into returned array when the
-        read has completed.
+        This function returns the number of bytes read by the underlying
+        ReadFile() function as an interger.
     """
-    ffi, library = dist.load()
-
     input_check("hFile", hFile, HANDLE)
+    input_check("lpBuffer", lpBuffer, bytearray)
     input_check("nNumberOfBytesToRead", nNumberOfBytesToRead, integer_types)
     input_check(
         "lpOverlapped", lpOverlapped,
         allowed_types=(NoneType, OVERLAPPED)
     )
 
-    readBuffer = bytearray(nNumberOfBytesToRead)
-    lpBuffer = ffi.from_buffer(readBuffer)
+    if len(lpBuffer) < nNumberOfBytesToRead:
+        raise InputError(
+            "ReadFile", None,
+            message="The length of lpBuffer is {} which is smaller than "
+                    "`nNumberOfBytesToRead` ({}).".format(
+                        len(lpBuffer), nNumberOfBytesToRead))
+
+    ffi, library = dist.load()
     bytes_read = ffi.new("LPDWORD")
     code = library.ReadFile(
-        wintype_to_cdata(hFile), lpBuffer, nNumberOfBytesToRead, bytes_read,
-        wintype_to_cdata(lpOverlapped)
+        wintype_to_cdata(hFile), ffi.from_buffer(lpBuffer),
+        nNumberOfBytesToRead, bytes_read, wintype_to_cdata(lpOverlapped)
     )
     error_check("ReadFile", code=code, expected=NON_ZERO)
-    return readBuffer
+    return bytes_read[0]
 
 
 def MoveFileEx(lpExistingFileName, lpNewFileName, dwFlags=None):

tests/test_kernel32/test_file.py

@@ -11,8 +11,7 @@
 
 from pywincffi.core import dist
 from pywincffi.dev.testutil import TestCase
-from pywincffi.exceptions import WindowsAPIError
-
+from pywincffi.exceptions import WindowsAPIError, InputError
 from pywincffi.kernel32 import file as _file  # used for mocks
 from pywincffi.kernel32 import (
     CreateFile, CloseHandle, MoveFileEx, WriteFile, FlushFileBuffers,
@@ -69,21 +68,33 @@ def _handle_to_read_file(self, path):
         self.addCleanup(CloseHandle, hFile)
         return hFile
 
-    def test_write_then_read_bytes_ascii(self):
-        path, written = self._create_file(b"test_write_then_read_bytes_ascii")
+    def test_lpBuffer_smaller_than_nNumberOfBytesToRead(self):
+        path, _ = self._create_file(b"")
         hFile = self._handle_to_read_file(path)
-        self.assertEqual(
-            ReadFile(hFile, written), b"test_write_then_read_bytes_ascii")
 
-    def test_write_then_read_null_bytes(self):
-        path, written = self._create_file(b"hello\x00world")
+        with self.assertRaisesRegex(
+            InputError, r".*The length of lpBuffer is.*"
+        ):
+            ReadFile(hFile, bytearray(0), 1)
+
+    def test_lpBuffer_equal_to_nNumberOfBytesToRead(self):
+        path, _ = self._create_file(b"")
         hFile = self._handle_to_read_file(path)
-        self.assertEqual(ReadFile(hFile, written), b"hello\x00world")
+        ReadFile(hFile, bytearray(1), 1)  # Should not raise exception
 
-    def test_write_then_read_partial(self):
-        path, _ = self._create_file(b"test_write_then_read_partial")
+    def test_read(self):
+        path, written = self._create_file(b"hello world")
+        self.assertEqual(written, 11)
         hFile = self._handle_to_read_file(path)
-        self.assertEqual(ReadFile(hFile, 4), b"test")
+        lpBuffer = bytearray(written)
+        read = ReadFile(hFile, lpBuffer, 5)
+        self.assertEqual(read, 5)
+        self.assertEqual(lpBuffer[:read], bytearray(b"hello"))
+
+        # The rest of the buffer is essentially unmanaged but let's be sure
+        # the remainder of the buffer has not been modified by ReadFile().
+        self.assertEqual(
+            lpBuffer[read:], bytearray(b"\x00\x00\x00\x00\x00\x00"))
 
 
 class TestMoveFileEx(TestCase):

tests/test_kernel32/test_pipe.py

@@ -55,9 +55,10 @@ def test_python_bytes(self):
 
         data = b"hello world"
         bytes_written = WriteFile(writer, data)
-        self.assertEqual(
-            ReadFile(reader, bytes_written),
-            b"hello world")
+        buffer = bytearray(bytes_written)
+        read = ReadFile(reader, buffer, bytes_written)
+        self.assertEqual(bytes_written, read)
+        self.assertEqual(buffer, bytearray(b"hello world"))
 
         _, library = dist.load()
         self.maybe_assert_last_error(library.ERROR_INVALID_HANDLE)
@@ -79,9 +80,11 @@ def test_peek_does_not_remove_data(self):
 
         data = b"hello world"
         data_written = WriteFile(writer, data)
-
+        buf = bytearray(data_written)
+        read = ReadFile(reader, buf, data_written)
+        self.assertEqual(read, data_written)
         PeekNamedPipe(reader, 0)
-        self.assertEqual(ReadFile(reader, data_written), data)
+        self.assertEqual(buf, bytearray(data))
         _, library = dist.load()
         self.maybe_assert_last_error(library.ERROR_INVALID_HANDLE)
 
@@ -125,7 +128,9 @@ def test_total_bytes_avail_after_read(self):
         bytes_written = WriteFile(writer, data)
 
         read_bytes = 7
-        ReadFile(reader, read_bytes)
+        buf = bytearray(read_bytes)
+        read = ReadFile(reader, buf, read_bytes)
+        self.assertEqual(read, read_bytes)
 
         result = PeekNamedPipe(reader, 0)
         self.assertEqual(