[Linux] cat/bcat utils refactoring (#2053)

5 files changed, 69 insertions, 57 deletions

diff --git a/psutil/_common.py b/psutil/_common.py
index 540d2f2d..8030e2e0 100644
--- a/psutil/_common.py
+++ b/psutil/_common.py
@@ -46,6 +46,7 @@ PY3 = sys.version_info[0] == 3
 PSUTIL_DEBUG = bool(os.getenv('PSUTIL_DEBUG', 0))
 if PSUTIL_DEBUG:
     import inspect
+_DEFAULT = object()
 
 __all__ = [
     # OS constants
@@ -72,6 +73,7 @@ __all__ = [
     'conn_tmap', 'deprecated_method', 'isfile_strict', 'memoize',
     'parse_environ_block', 'path_exists_strict', 'usage_percent',
     'supports_ipv6', 'sockfam_to_enum', 'socktype_to_enum', "wrap_numbers",
+    'open_text', 'open_binary', 'cat', 'bcat',
     'bytes2human', 'conn_to_ntuple', 'debug',
     # shell utils
     'hilite', 'term_supports_colors', 'print_color',
@@ -727,6 +729,28 @@ def open_text(fname, **kwargs):
     return open(fname, "rt", **kwargs)
 
 
+def cat(fname, fallback=_DEFAULT, _open=open_text):
+    """Read entire file content and return it as a string. File is
+    opened in text mode. If specified, `fallback` is the value
+    returned in case of error, either if the file does not exist or
+    it can't be read().
+    """
+    if fallback is _DEFAULT:
+        with _open(fname) as f:
+            return f.read()
+    else:
+        try:
+            with _open(fname) as f:
+                return f.read()
+        except (IOError, OSError):
+            return fallback
+
+
+def bcat(fname, fallback=_DEFAULT):
+    """Same as above but opens file in binary mode."""
+    return cat(fname, fallback=fallback, _open=open_binary)
+
+
 def bytes2human(n, format="%(value).1f%(symbol)s"):
     """Used by various scripts. See:
     http://goo.gl/zeJZl
diff --git a/psutil/_pslinux.py b/psutil/_pslinux.py
index ab1b6f1b..9291d8c2 100644
--- a/psutil/_pslinux.py
+++ b/psutil/_pslinux.py
@@ -31,6 +31,8 @@ from ._common import NIC_DUPLEX_UNKNOWN
 from ._common import AccessDenied
 from ._common import NoSuchProcess
 from ._common import ZombieProcess
+from ._common import bcat
+from ._common import cat
 from ._common import debug
 from ._common import decode
 from ._common import get_procfs_path
@@ -78,7 +80,6 @@ POWER_SUPPLY_PATH = "/sys/class/power_supply"
 HAS_SMAPS = os.path.exists('/proc/%s/smaps' % os.getpid())
 HAS_PROC_IO_PRIORITY = hasattr(cext, "proc_ioprio_get")
 HAS_CPU_AFFINITY = hasattr(cext, "proc_cpu_affinity_get")
-_DEFAULT = object()
 
 # Number of clock ticks per second
 CLOCK_TICKS = os.sysconf("SC_CLK_TCK")
@@ -283,22 +284,6 @@ def set_scputimes_ntuple(procfs_path):
     scputimes = namedtuple('scputimes', fields)
 
 
-def cat(fname, fallback=_DEFAULT, binary=True):
-    """Return file content.
-    fallback: the value returned in case the file does not exist or
-              cannot be read
-    binary: whether to open the file in binary or text mode.
-    """
-    try:
-        with open_binary(fname) if binary else open_text(fname) as f:
-            return f.read().strip()
-    except (IOError, OSError):
-        if fallback is not _DEFAULT:
-            return fallback
-        else:
-            raise
-
-
 try:
     set_scputimes_ntuple("/proc")
 except Exception:  # pragma: no cover
@@ -751,17 +736,17 @@ if os.path.exists("/sys/devices/system/cpu/cpufreq/policy0") or \
                 # https://github.com/giampaolo/psutil/issues/1851
                 curr = cpuinfo_freqs[i] * 1000
             else:
-                curr = cat(pjoin(path, "scaling_cur_freq"), fallback=None)
+                curr = bcat(pjoin(path, "scaling_cur_freq"), fallback=None)
             if curr is None:
                 # Likely an old RedHat, see:
                 # https://github.com/giampaolo/psutil/issues/1071
-                curr = cat(pjoin(path, "cpuinfo_cur_freq"), fallback=None)
+                curr = bcat(pjoin(path, "cpuinfo_cur_freq"), fallback=None)
                 if curr is None:
                     raise NotImplementedError(
                         "can't find current frequency file")
             curr = int(curr) / 1000
-            max_ = int(cat(pjoin(path, "scaling_max_freq"))) / 1000
-            min_ = int(cat(pjoin(path, "scaling_min_freq"))) / 1000
+            max_ = int(bcat(pjoin(path, "scaling_max_freq"))) / 1000
+            min_ = int(bcat(pjoin(path, "scaling_min_freq"))) / 1000
             ret.append(_common.scpufreq(curr, min_, max_))
         return ret
 
@@ -1349,9 +1334,9 @@ def sensors_temperatures():
     for base in basenames:
         try:
             path = base + '_input'
-            current = float(cat(path)) / 1000.0
+            current = float(bcat(path)) / 1000.0
             path = os.path.join(os.path.dirname(base), 'name')
-            unit_name = cat(path, binary=False)
+            unit_name = cat(path).strip()
         except (IOError, OSError, ValueError):
             # A lot of things can go wrong here, so let's just skip the
             # whole entry. Sure thing is Linux's /sys/class/hwmon really
@@ -1363,9 +1348,9 @@ def sensors_temperatures():
             # https://github.com/giampaolo/psutil/issues/1323
             continue
 
-        high = cat(base + '_max', fallback=None)
-        critical = cat(base + '_crit', fallback=None)
-        label = cat(base + '_label', fallback='', binary=False)
+        high = bcat(base + '_max', fallback=None)
+        critical = bcat(base + '_crit', fallback=None)
+        label = cat(base + '_label', fallback='').strip()
 
         if high is not None:
             try:
@@ -1388,9 +1373,9 @@ def sensors_temperatures():
         for base in basenames:
             try:
                 path = os.path.join(base, 'temp')
-                current = float(cat(path)) / 1000.0
+                current = float(bcat(path)) / 1000.0
                 path = os.path.join(base, 'type')
-                unit_name = cat(path, binary=False)
+                unit_name = cat(path).strip()
             except (IOError, OSError, ValueError) as err:
                 debug(err)
                 continue
@@ -1402,13 +1387,13 @@ def sensors_temperatures():
             high = None
             for trip_point in trip_points:
                 path = os.path.join(base, trip_point + "_type")
-                trip_type = cat(path, fallback='', binary=False)
+                trip_type = cat(path, fallback='').strip()
                 if trip_type == 'critical':
-                    critical = cat(os.path.join(base, trip_point + "_temp"),
-                                   fallback=None)
+                    critical = bcat(os.path.join(base, trip_point + "_temp"),
+                                    fallback=None)
                 elif trip_type == 'high':
-                    high = cat(os.path.join(base, trip_point + "_temp"),
-                               fallback=None)
+                    high = bcat(os.path.join(base, trip_point + "_temp"),
+                                fallback=None)
 
                 if high is not None:
                     try:
@@ -1446,13 +1431,12 @@ def sensors_fans():
     basenames = sorted(set([x.split('_')[0] for x in basenames]))
     for base in basenames:
         try:
-            current = int(cat(base + '_input'))
+            current = int(bcat(base + '_input'))
         except (IOError, OSError) as err:
             debug(err)
             continue
-        unit_name = cat(os.path.join(os.path.dirname(base), 'name'),
-                        binary=False)
-        label = cat(base + '_label', fallback='', binary=False)
+        unit_name = cat(os.path.join(os.path.dirname(base), 'name'))
+        label = cat(base + '_label', fallback='')
         ret[unit_name].append(_common.sfan(label, current))
 
     return dict(ret)
@@ -1467,12 +1451,12 @@ def sensors_battery():
     """
     null = object()
 
-    def multi_cat(*paths):
+    def multi_bcat(*paths):
         """Attempt to read the content of multiple files which may
         not exist. If none of them exist return None.
         """
         for path in paths:
-            ret = cat(path, fallback=null)
+            ret = bcat(path, fallback=null)
             if ret != null:
                 try:
                     return int(ret)
@@ -1490,16 +1474,16 @@ def sensors_battery():
     root = os.path.join(POWER_SUPPLY_PATH, sorted(bats)[0])
 
     # Base metrics.
-    energy_now = multi_cat(
+    energy_now = multi_bcat(
         root + "/energy_now",
         root + "/charge_now")
-    power_now = multi_cat(
+    power_now = multi_bcat(
         root + "/power_now",
         root + "/current_now")
-    energy_full = multi_cat(
+    energy_full = multi_bcat(
         root + "/energy_full",
         root + "/charge_full")
-    time_to_empty = multi_cat(root + "/time_to_empty_now")
+    time_to_empty = multi_bcat(root + "/time_to_empty_now")
 
     # Percent. If we have energy_full the percentage will be more
     # accurate compared to reading /capacity file (float vs. int).
@@ -1517,13 +1501,13 @@ def sensors_battery():
     # Note: AC0 is not always available and sometimes (e.g. CentOS7)
     # it's called "AC".
     power_plugged = None
-    online = multi_cat(
+    online = multi_bcat(
         os.path.join(POWER_SUPPLY_PATH, "AC0/online"),
         os.path.join(POWER_SUPPLY_PATH, "AC/online"))
     if online is not None:
         power_plugged = online == 1
     else:
-        status = cat(root + "/status", fallback="", binary=False).lower()
+        status = cat(root + "/status", fallback="").strip().lower()
         if status == "discharging":
             power_plugged = False
         elif status in ("charging", "full"):
@@ -1700,8 +1684,7 @@ class Process(object):
         The return value is cached in case oneshot() ctx manager is
         in use.
         """
-        with open_binary("%s/%s/stat" % (self._procfs_path, self.pid)) as f:
-            data = f.read()
+        data = bcat("%s/%s/stat" % (self._procfs_path, self.pid))
         # Process name is between parentheses. It can contain spaces and
         # other parentheses. This is taken into account by looking for
         # the first occurrence of "(" and the last occurence of ")".
diff --git a/psutil/tests/test_linux.py b/psutil/tests/test_linux.py
index f9d092d9..b69840be 100755
--- a/psutil/tests/test_linux.py
+++ b/psutil/tests/test_linux.py
@@ -2250,15 +2250,6 @@ class TestUtils(PsutilTestCase):
             self.assertEqual(psutil._psplatform.readlink("bar"), "foo")
             assert m.called
 
-    def test_cat(self):
-        testfn = self.get_testfn()
-        with open(testfn, "wt") as f:
-            f.write("foo ")
-        self.assertEqual(psutil._psplatform.cat(testfn, binary=False), "foo")
-        self.assertEqual(psutil._psplatform.cat(testfn, binary=True), b"foo")
-        self.assertEqual(
-            psutil._psplatform.cat(testfn + '??', fallback="bar"), "bar")
-
 
 if __name__ == '__main__':
     from psutil.tests.runner import run_from_name
diff --git a/psutil/tests/test_misc.py b/psutil/tests/test_misc.py
index a13c295d..aa30cbd6 100755
--- a/psutil/tests/test_misc.py
+++ b/psutil/tests/test_misc.py
@@ -23,6 +23,8 @@ import psutil.tests
 from psutil import LINUX
 from psutil import POSIX
 from psutil import WINDOWS
+from psutil._common import bcat
+from psutil._common import cat
 from psutil._common import debug
 from psutil._common import isfile_strict
 from psutil._common import memoize
@@ -31,6 +33,7 @@ from psutil._common import parse_environ_block
 from psutil._common import supports_ipv6
 from psutil._common import wrap_numbers
 from psutil._compat import PY3
+from psutil._compat import FileNotFoundError
 from psutil._compat import redirect_stderr
 from psutil.tests import APPVEYOR
 from psutil.tests import CI_TESTING
@@ -451,6 +454,17 @@ class TestCommonModule(PsutilTestCase):
         self.assertIn("no such file", msg)
         self.assertIn("/foo", msg)
 
+    def test_cat_bcat(self):
+        testfn = self.get_testfn()
+        with open(testfn, "wt") as f:
+            f.write("foo")
+        self.assertEqual(cat(testfn), "foo")
+        self.assertEqual(bcat(testfn), b"foo")
+        self.assertRaises(FileNotFoundError, cat, testfn + '-invalid')
+        self.assertRaises(FileNotFoundError, bcat, testfn + '-invalid')
+        self.assertEqual(cat(testfn + '-invalid', fallback="bar"), "bar")
+        self.assertEqual(bcat(testfn + '-invalid', fallback="bar"), "bar")
+
 
 # ===================================================================
 # --- Tests for wrap_numbers() function.
diff --git a/psutil/tests/test_system.py b/psutil/tests/test_system.py
index d98ec5c3..ed328d01 100755
--- a/psutil/tests/test_system.py
+++ b/psutil/tests/test_system.py
@@ -184,7 +184,7 @@ class TestProcessAPIs(PsutilTestCase):
                 # in case the process disappeared in meantime fail only
                 # if it is no longer in psutil.pids()
                 time.sleep(.1)
-                self.assertIn(pid, psutil.pids())
+                self.assertNotIn(pid, psutil.pids())
         pids = range(max(pids) + 5000, max(pids) + 6000)
         for pid in pids:
             self.assertFalse(psutil.pid_exists(pid), msg=pid)