properly check OSError.winerrorwinerr-handling

2 files changed, 50 insertions, 49 deletions

diff --git a/psutil/_pswindows.py b/psutil/_pswindows.py
index 6687770c..260651d1 100644
--- a/psutil/_pswindows.py
+++ b/psutil/_pswindows.py
@@ -76,10 +76,6 @@ __extra__all__ = [
 # =====================================================================
 
 CONN_DELETE_TCB = "DELETE_TCB"
-ACCESS_DENIED_ERRSET = frozenset([errno.EPERM, errno.EACCES,
-                                  cext.ERROR_ACCESS_DENIED])
-NO_SUCH_SERVICE_ERRSET = frozenset([cext.ERROR_INVALID_NAME,
-                                    cext.ERROR_SERVICE_DOES_NOT_EXIST])
 HAS_PROC_IO_PRIORITY = hasattr(cext, "proc_io_priority_get")
 
 
@@ -533,14 +529,14 @@ class WindowsService(object):
         """
         try:
             yield
-        except WindowsError as err:
-            if err.errno in ACCESS_DENIED_ERRSET:
+        except OSError as err:
+            if is_permission_err(err):
                 raise AccessDenied(
                     pid=None, name=self._name,
                     msg="service %r is not querable (not enough privileges)" %
                         self._name)
-            elif err.errno in NO_SUCH_SERVICE_ERRSET or \
-                    err.winerror in NO_SUCH_SERVICE_ERRSET:
+            elif err.winerror in (cext.ERROR_INVALID_NAME,
+                                  cext.ERROR_SERVICE_DOES_NOT_EXIST):
                 raise NoSuchProcess(
                     pid=None, name=self._name,
                     msg="service %r does not exist)" % self._name)
@@ -657,20 +653,31 @@ pid_exists = cext.pid_exists
 ppid_map = cext.ppid_map  # used internally by Process.children()
 
 
+def is_permission_err(exc):
+    """Return True if this is a permission error."""
+    assert isinstance(exc, OSError), exc
+    return exc.errno in (errno.EPERM, errno.EACCES) or \
+        exc.winerror == cext.ERROR_ACCESS_DENIED
+
+
+def convert_oserror(exc, pid=None, name=None):
+    """Convert OSError into NoSuchProcess or AccessDenied."""
+    assert isinstance(exc, OSError), exc
+    if is_permission_err(exc):
+        return AccessDenied(pid=pid, name=name)
+    if exc.errno == errno.ESRCH:
+        return NoSuchProcess(pid=pid, name=name)
+    raise exc
+
+
 def wrap_exceptions(fun):
-    """Decorator which translates bare OSError and WindowsError
-    exceptions into NoSuchProcess and AccessDenied.
-    """
+    """Decorator which converts OSError into NoSuchProcess or AccessDenied."""
     @functools.wraps(fun)
     def wrapper(self, *args, **kwargs):
         try:
             return fun(self, *args, **kwargs)
         except OSError as err:
-            if err.errno in ACCESS_DENIED_ERRSET:
-                raise AccessDenied(self.pid, self._name)
-            if err.errno == errno.ESRCH:
-                raise NoSuchProcess(self.pid, self._name)
-            raise
+            raise convert_oserror(err, pid=self.pid, name=self._name)
     return wrapper
 
 
@@ -744,7 +751,7 @@ class Process(object):
             try:
                 ret = cext.proc_cmdline(self.pid, use_peb=True)
             except OSError as err:
-                if err.errno in ACCESS_DENIED_ERRSET:
+                if is_permission_err(err):
                     ret = cext.proc_cmdline(self.pid, use_peb=False)
                 else:
                     raise
@@ -772,7 +779,7 @@ class Process(object):
         try:
             return cext.proc_memory_info(self.pid)
         except OSError as err:
-            if err.errno in ACCESS_DENIED_ERRSET:
+            if is_permission_err(err):
                 # TODO: the C ext can probably be refactored in order
                 # to get this from cext.proc_info()
                 info = self.oneshot_info()
@@ -813,11 +820,7 @@ class Process(object):
         except OSError as err:
             # XXX - can't use wrap_exceptions decorator as we're
             # returning a generator; probably needs refactoring.
-            if err.errno in ACCESS_DENIED_ERRSET:
-                raise AccessDenied(self.pid, self._name)
-            if err.errno == errno.ESRCH:
-                raise NoSuchProcess(self.pid, self._name)
-            raise
+            raise convert_oserror(err, self.pid, self._name)
         else:
             for addr, perm, path, rss in raw:
                 path = convert_dos_path(path)
@@ -893,7 +896,7 @@ class Process(object):
         try:
             return cext.proc_create_time(self.pid)
         except OSError as err:
-            if err.errno in ACCESS_DENIED_ERRSET:
+            if is_permission_err(err):
                 return self.oneshot_info()[pinfo_map['create_time']]
             raise
 
@@ -915,12 +918,11 @@ class Process(object):
         try:
             user, system = cext.proc_cpu_times(self.pid)
         except OSError as err:
-            if err.errno in ACCESS_DENIED_ERRSET:
-                info = self.oneshot_info()
-                user = info[pinfo_map['user_time']]
-                system = info[pinfo_map['kernel_time']]
-            else:
+            if not is_permission_err(err):
                 raise
+            info = self.oneshot_info()
+            user = info[pinfo_map['user_time']]
+            system = info[pinfo_map['kernel_time']]
         # Children user/system times are not retrievable (set to 0).
         return _common.pcputimes(user, system, 0.0, 0.0)
 
@@ -996,18 +998,17 @@ class Process(object):
         try:
             ret = cext.proc_io_counters(self.pid)
         except OSError as err:
-            if err.errno in ACCESS_DENIED_ERRSET:
-                info = self.oneshot_info()
-                ret = (
-                    info[pinfo_map['io_rcount']],
-                    info[pinfo_map['io_wcount']],
-                    info[pinfo_map['io_rbytes']],
-                    info[pinfo_map['io_wbytes']],
-                    info[pinfo_map['io_count_others']],
-                    info[pinfo_map['io_bytes_others']],
-                )
-            else:
+            if not is_permission_err(err):
                 raise
+            info = self.oneshot_info()
+            ret = (
+                info[pinfo_map['io_rcount']],
+                info[pinfo_map['io_wcount']],
+                info[pinfo_map['io_rbytes']],
+                info[pinfo_map['io_wbytes']],
+                info[pinfo_map['io_count_others']],
+                info[pinfo_map['io_bytes_others']],
+            )
         return pio(*ret)
 
     @wrap_exceptions
@@ -1055,7 +1056,7 @@ class Process(object):
         try:
             return cext.proc_num_handles(self.pid)
         except OSError as err:
-            if err.errno in ACCESS_DENIED_ERRSET:
+            if is_permission_err(err):
                 return self.oneshot_info()[pinfo_map['num_handles']]
             raise
 
diff --git a/psutil/tests/test_windows.py b/psutil/tests/test_windows.py
index a3a6b61d..70c99b4b 100755
--- a/psutil/tests/test_windows.py
+++ b/psutil/tests/test_windows.py
@@ -664,17 +664,15 @@ class TestDualProcessImplementation(unittest.TestCase):
             assert fun.called
 
     def test_cmdline(self):
-        from psutil._pswindows import ACCESS_DENIED_ERRSET
+        from psutil._pswindows import convert_oserror
         for pid in psutil.pids():
             try:
                 a = cext.proc_cmdline(pid, use_peb=True)
                 b = cext.proc_cmdline(pid, use_peb=False)
             except OSError as err:
-                if err.errno in ACCESS_DENIED_ERRSET:
-                    pass
-                elif err.errno == errno.ESRCH:
-                    pass  # NSP
-                else:
+                err = convert_oserror(err)
+                if not isinstance(err, (psutil.AccessDenied,
+                                        psutil.NoSuchProcess)):
                     raise
             else:
                 self.assertEqual(a, b)
@@ -837,7 +835,8 @@ class TestServices(unittest.TestCase):
         # test NoSuchProcess
         service = psutil.win_service_get(name)
         exc = WindowsError(
-            psutil._psplatform.cext.ERROR_SERVICE_DOES_NOT_EXIST, "")
+            0, "", 0,
+            psutil._psplatform.cext.ERROR_SERVICE_DOES_NOT_EXIST)
         with mock.patch("psutil._psplatform.cext.winservice_query_status",
                         side_effect=exc):
             self.assertRaises(psutil.NoSuchProcess, service.status)
@@ -847,7 +846,8 @@ class TestServices(unittest.TestCase):
 
         # test AccessDenied
         exc = WindowsError(
-            psutil._psplatform.cext.ERROR_ACCESS_DENIED, "")
+            0, "", 0,
+            psutil._psplatform.cext.ERROR_ACCESS_DENIED)
         with mock.patch("psutil._psplatform.cext.winservice_query_status",
                         side_effect=exc):
             self.assertRaises(psutil.AccessDenied, service.status)