From: Daira Hopwood <daira@jacaranda.org>
Date: Tue, 27 Oct 2015 14:47:11 +0000 (+0000)
Subject: Improve all of the Windows-specific error reporting.
X-Git-Url: https://git.rkrishnan.org/components/%22news.html/configuration.txt?a=commitdiff_plain;h=e6a027b5ac522b615f5b4266d5e9f6e015b5b589;p=tahoe-lafs%2Ftahoe-lafs.git

Improve all of the Windows-specific error reporting.
Also make the Windows function declarations more readable and consistent.

Signed-off-by: Daira Hopwood <daira@jacaranda.org>
---

diff --git a/src/allmydata/util/fileutil.py b/src/allmydata/util/fileutil.py
index 4ea1263e..1133c261 100644
--- a/src/allmydata/util/fileutil.py
+++ b/src/allmydata/util/fileutil.py
@@ -345,10 +345,9 @@ have_GetDiskFreeSpaceExW = False
 if sys.platform == "win32":
     # <http://msdn.microsoft.com/en-us/library/windows/desktop/ms683188%28v=vs.85%29.aspx>
     GetEnvironmentVariableW = WINFUNCTYPE(
-        DWORD,
-          LPCWSTR, LPWSTR, DWORD,
+        DWORD,  LPCWSTR, LPWSTR, DWORD,
         use_last_error=True
-      )(("GetEnvironmentVariableW", windll.kernel32))
+    )(("GetEnvironmentVariableW", windll.kernel32))
 
     try:
         # <http://msdn.microsoft.com/en-us/library/aa383742%28v=VS.85%29.aspx>
@@ -356,10 +355,9 @@ if sys.platform == "win32":
 
         # <http://msdn.microsoft.com/en-us/library/aa364937%28VS.85%29.aspx>
         GetDiskFreeSpaceExW = WINFUNCTYPE(
-            BOOL,
-              LPCWSTR, PULARGE_INTEGER, PULARGE_INTEGER, PULARGE_INTEGER,
+            BOOL,  LPCWSTR, PULARGE_INTEGER, PULARGE_INTEGER, PULARGE_INTEGER,
             use_last_error=True
-          )(("GetDiskFreeSpaceExW", windll.kernel32))
+        )(("GetDiskFreeSpaceExW", windll.kernel32))
 
         have_GetDiskFreeSpaceExW = True
     except Exception:
@@ -407,8 +405,8 @@ def windows_getenv(name):
         err = get_last_error()
         if err == ERROR_ENVVAR_NOT_FOUND:
             return None
-        raise OSError("Windows error %d attempting to read size of environment variable %r"
-                      % (err, name))
+        raise OSError("WinError: %s\n attempting to read size of environment variable %r"
+                      % (WinError(err), name))
     if n == 1:
         # Avoid an ambiguity between a zero-length string and an error in the return value of the
         # call to GetEnvironmentVariableW below.
@@ -420,8 +418,8 @@ def windows_getenv(name):
         err = get_last_error()
         if err == ERROR_ENVVAR_NOT_FOUND:
             return None
-        raise OSError("Windows error %d attempting to read environment variable %r"
-                      % (err, name))
+        raise OSError("WinError: %s\n attempting to read environment variable %r"
+                      % (WinError(err), name))
     if retval >= n:
         raise OSError("Unexpected result %d (expected less than %d) from GetEnvironmentVariableW attempting to read environment variable %r"
                       % (retval, n, name))
@@ -463,8 +461,8 @@ def get_disk_stats(whichdir, reserved_space=0):
                                                byref(n_total),
                                                byref(n_free_for_root))
         if retval == 0:
-            raise OSError("Windows error %d attempting to get disk statistics for %r"
-                          % (get_last_error(), whichdir))
+            raise OSError("WinError: %s\n attempting to get disk statistics for %r"
+                          % (WinError(get_last_error()), whichdir))
         free_for_nonroot = n_free_for_nonroot.value
         total            = n_total.value
         free_for_root    = n_free_for_root.value
@@ -523,8 +521,10 @@ def get_available_space(whichdir, reserved_space):
 
 if sys.platform == "win32":
     # <http://msdn.microsoft.com/en-us/library/aa363858%28v=vs.85%29.aspx>
-    CreateFileW = WINFUNCTYPE(HANDLE, LPCWSTR, DWORD, DWORD, LPVOID, DWORD, DWORD, HANDLE) \
-                      (("CreateFileW", windll.kernel32))
+    CreateFileW = WINFUNCTYPE(
+        HANDLE,  LPCWSTR, DWORD, DWORD, LPVOID, DWORD, DWORD, HANDLE,
+        use_last_error=True
+    )(("CreateFileW", windll.kernel32))
 
     GENERIC_WRITE        = 0x40000000
     FILE_SHARE_READ      = 0x00000001
@@ -533,10 +533,16 @@ if sys.platform == "win32":
     INVALID_HANDLE_VALUE = 0xFFFFFFFF
 
     # <http://msdn.microsoft.com/en-us/library/aa364439%28v=vs.85%29.aspx>
-    FlushFileBuffers = WINFUNCTYPE(BOOL, HANDLE)(("FlushFileBuffers", windll.kernel32))
+    FlushFileBuffers = WINFUNCTYPE(
+        BOOL,  HANDLE,
+        use_last_error=True
+    )(("FlushFileBuffers", windll.kernel32))
 
     # <http://msdn.microsoft.com/en-us/library/ms724211%28v=vs.85%29.aspx>
-    CloseHandle = WINFUNCTYPE(BOOL, HANDLE)(("CloseHandle", windll.kernel32))
+    CloseHandle = WINFUNCTYPE(
+        BOOL,  HANDLE,
+        use_last_error=True
+    )(("CloseHandle", windll.kernel32))
 
     # <http://social.msdn.microsoft.com/forums/en-US/netfxbcl/thread/4465cafb-f4ed-434f-89d8-c85ced6ffaa8/>
     def flush_volume(path):
@@ -551,10 +557,10 @@ if sys.platform == "win32":
                               None
                              )
         if hVolume == INVALID_HANDLE_VALUE:
-            raise WinError()
+            raise WinError(get_last_error())
 
         if FlushFileBuffers(hVolume) == 0:
-            raise WinError()
+            raise WinError(get_last_error())
 
         CloseHandle(hVolume)
 else:
@@ -577,10 +583,9 @@ def reraise(wrapper):
 if sys.platform == "win32":
     # <https://msdn.microsoft.com/en-us/library/windows/desktop/aa365512%28v=vs.85%29.aspx>
     ReplaceFileW = WINFUNCTYPE(
-        BOOL,
-          LPCWSTR, LPCWSTR, LPCWSTR, DWORD, LPVOID, LPVOID,
+        BOOL,  LPCWSTR, LPCWSTR, LPCWSTR, DWORD, LPVOID, LPVOID,
         use_last_error=True
-      )(("ReplaceFileW", windll.kernel32))
+    )(("ReplaceFileW", windll.kernel32))
 
     REPLACEFILE_IGNORE_MERGE_ERRORS = 0x00000002
 
diff --git a/src/allmydata/windows/fixups.py b/src/allmydata/windows/fixups.py
index eaf5d5eb..490e0850 100644
--- a/src/allmydata/windows/fixups.py
+++ b/src/allmydata/windows/fixups.py
@@ -9,13 +9,18 @@ def initialize():
     done = True
 
     import codecs, re
-    from ctypes import WINFUNCTYPE, windll, POINTER, byref, c_int
+    from ctypes import WINFUNCTYPE, WinError, windll, POINTER, byref, c_int, get_last_error
     from ctypes.wintypes import BOOL, HANDLE, DWORD, UINT, LPWSTR, LPCWSTR, LPVOID
+
     from allmydata.util import log
     from allmydata.util.encodingutil import canonical_encoding
 
     # <https://msdn.microsoft.com/en-us/library/ms680621%28VS.85%29.aspx>
-    SetErrorMode = WINFUNCTYPE(UINT, UINT)(("SetErrorMode", windll.kernel32))
+    SetErrorMode = WINFUNCTYPE(
+        UINT,  UINT,
+        use_last_error=True
+    )(("SetErrorMode", windll.kernel32))
+
     SEM_FAILCRITICALERRORS = 0x0001
     SEM_NOOPENFILEERRORBOX = 0x8000
 
@@ -50,13 +55,27 @@ def initialize():
         # <https://msdn.microsoft.com/en-us/library/ms683167(VS.85).aspx>
         # BOOL WINAPI GetConsoleMode(HANDLE hConsole, LPDWORD lpMode);
 
-        GetStdHandle = WINFUNCTYPE(HANDLE, DWORD)(("GetStdHandle", windll.kernel32))
+        GetStdHandle = WINFUNCTYPE(
+            HANDLE,  DWORD,
+            use_last_error=True
+        )(("GetStdHandle", windll.kernel32))
+
         STD_OUTPUT_HANDLE = DWORD(-11)
         STD_ERROR_HANDLE  = DWORD(-12)
-        GetFileType = WINFUNCTYPE(DWORD, DWORD)(("GetFileType", windll.kernel32))
+
+        GetFileType = WINFUNCTYPE(
+            DWORD,  DWORD,
+            use_last_error=True
+        )(("GetFileType", windll.kernel32))
+
         FILE_TYPE_CHAR   = 0x0002
         FILE_TYPE_REMOTE = 0x8000
-        GetConsoleMode = WINFUNCTYPE(BOOL, HANDLE, POINTER(DWORD))(("GetConsoleMode", windll.kernel32))
+
+        GetConsoleMode = WINFUNCTYPE(
+            BOOL,  HANDLE, POINTER(DWORD),
+            use_last_error=True
+        )(("GetConsoleMode", windll.kernel32))
+
         INVALID_HANDLE_VALUE = DWORD(-1).value
 
         def not_a_console(handle):
@@ -88,11 +107,14 @@ def initialize():
                 real_stderr = False
 
         if real_stdout or real_stderr:
+            # <https://msdn.microsoft.com/en-us/library/windows/desktop/ms687401%28v=vs.85%29.aspx>
             # BOOL WINAPI WriteConsoleW(HANDLE hOutput, LPWSTR lpBuffer, DWORD nChars,
             #                           LPDWORD lpCharsWritten, LPVOID lpReserved);
 
-            WriteConsoleW = WINFUNCTYPE(BOOL, HANDLE, LPWSTR, DWORD, POINTER(DWORD), LPVOID) \
-                                (("WriteConsoleW", windll.kernel32))
+            WriteConsoleW = WINFUNCTYPE(
+                BOOL,  HANDLE, LPWSTR, DWORD, POINTER(DWORD), LPVOID,
+                use_last_error=True
+            )(("WriteConsoleW", windll.kernel32))
 
             class UnicodeOutput:
                 def __init__(self, hConsole, stream, fileno, name):
@@ -139,8 +161,10 @@ def initialize():
                                 # There is a shorter-than-documented limitation on the length of the string
                                 # passed to WriteConsoleW (see #1232).
                                 retval = WriteConsoleW(self._hConsole, text, min(remaining, 10000), byref(n), None)
-                                if retval == 0 or n.value == 0:
-                                    raise IOError("WriteConsoleW returned %r, n.value = %r" % (retval, n.value))
+                                if retval == 0:
+                                    raise IOError("WriteConsoleW failed with WinError: %s" % (WinError(get_last_error()),))
+                                if n.value == 0:
+                                    raise IOError("WriteConsoleW returned %r, n.value = 0" % (retval,))
                                 remaining -= n.value
                                 if remaining == 0: break
                                 text = text[n.value:]
@@ -169,12 +193,23 @@ def initialize():
         _complain("exception %r while fixing up sys.stdout and sys.stderr" % (e,))
 
     # This works around <http://bugs.python.org/issue2128>.
-    GetCommandLineW = WINFUNCTYPE(LPWSTR)(("GetCommandLineW", windll.kernel32))
-    CommandLineToArgvW = WINFUNCTYPE(POINTER(LPWSTR), LPCWSTR, POINTER(c_int)) \
-                            (("CommandLineToArgvW", windll.shell32))
+
+    # <https://msdn.microsoft.com/en-us/library/windows/desktop/ms683156%28v=vs.85%29.aspx>
+    GetCommandLineW = WINFUNCTYPE(
+        LPWSTR,
+        use_last_error=True
+    )(("GetCommandLineW", windll.kernel32))
+
+    # <https://msdn.microsoft.com/en-us/library/windows/desktop/bb776391%28v=vs.85%29.aspx>
+    CommandLineToArgvW = WINFUNCTYPE(
+        POINTER(LPWSTR),  LPCWSTR, POINTER(c_int),
+        use_last_error=True
+    )(("CommandLineToArgvW", windll.shell32))
 
     argc = c_int(0)
     argv_unicode = CommandLineToArgvW(GetCommandLineW(), byref(argc))
+    if argv_unicode is None:
+        raise WinError(get_last_error())
 
     # Because of <http://bugs.python.org/issue8775> (and similar limitations in
     # twisted), the 'bin/tahoe' script cannot invoke us with the actual Unicode arguments.
diff --git a/src/allmydata/windows/inotify.py b/src/allmydata/windows/inotify.py
index 1a719f9a..c5099ca4 100644
--- a/src/allmydata/windows/inotify.py
+++ b/src/allmydata/windows/inotify.py
@@ -23,15 +23,18 @@ from allmydata.util.encodingutil import quote_output
 from allmydata.util import log, fileutil
 from allmydata.util.pollmixin import PollMixin
 
-from ctypes import WINFUNCTYPE, WinError, windll, POINTER, byref, create_string_buffer, addressof
+from ctypes import WINFUNCTYPE, WinError, windll, POINTER, byref, create_string_buffer, \
+    addressof, get_last_error
 from ctypes.wintypes import BOOL, HANDLE, DWORD, LPCWSTR, LPVOID
 
 # <http://msdn.microsoft.com/en-us/library/gg258116%28v=vs.85%29.aspx>
 FILE_LIST_DIRECTORY              = 1
 
 # <http://msdn.microsoft.com/en-us/library/aa363858%28v=vs.85%29.aspx>
-CreateFileW = WINFUNCTYPE(HANDLE, LPCWSTR, DWORD, DWORD, LPVOID, DWORD, DWORD, HANDLE) \
-                  (("CreateFileW", windll.kernel32))
+CreateFileW = WINFUNCTYPE(
+    HANDLE,  LPCWSTR, DWORD, DWORD, LPVOID, DWORD, DWORD, HANDLE,
+    use_last_error=True
+)(("CreateFileW", windll.kernel32))
 
 FILE_SHARE_READ                  = 0x00000001
 FILE_SHARE_WRITE                 = 0x00000002
@@ -42,11 +45,16 @@ OPEN_EXISTING                    = 3
 FILE_FLAG_BACKUP_SEMANTICS       = 0x02000000
 
 # <http://msdn.microsoft.com/en-us/library/ms724211%28v=vs.85%29.aspx>
-CloseHandle = WINFUNCTYPE(BOOL, HANDLE)(("CloseHandle", windll.kernel32))
+CloseHandle = WINFUNCTYPE(
+    BOOL,  HANDLE,
+    use_last_error=True
+)(("CloseHandle", windll.kernel32))
 
 # <http://msdn.microsoft.com/en-us/library/aa365465%28v=vs.85%29.aspx>
-ReadDirectoryChangesW = WINFUNCTYPE(BOOL, HANDLE, LPVOID, DWORD, BOOL, DWORD, POINTER(DWORD), LPVOID, LPVOID) \
-                            (("ReadDirectoryChangesW", windll.kernel32))
+ReadDirectoryChangesW = WINFUNCTYPE(
+    BOOL,  HANDLE, LPVOID, DWORD, BOOL, DWORD, POINTER(DWORD), LPVOID, LPVOID,
+    use_last_error=True
+)(("ReadDirectoryChangesW", windll.kernel32))
 
 FILE_NOTIFY_CHANGE_FILE_NAME     = 0x00000001
 FILE_NOTIFY_CHANGE_DIR_NAME      = 0x00000002
@@ -121,7 +129,7 @@ class FileNotifyInformation(object):
                                   None   # NULL -> no completion routine
                                  )
         if r == 0:
-            raise WinError()
+            raise WinError(get_last_error())
         self.data = self.buffer.raw[:bytes_returned.value]
 
     def __iter__(self):
@@ -157,8 +165,8 @@ def _open_directory(path_u):
                              None                         # no template file
                             )
     if hDirectory == INVALID_HANDLE_VALUE:
-        e = WinError()
-        raise OSError("Opening directory %s gave Windows error %r: %s" % (quote_output(path_u), e.args[0], e.args[1]))
+        e = WinError(get_last_error())
+        raise OSError("Opening directory %s gave WinError: %s" % (quote_output(path_u), e))
     return hDirectory