From a873a23e0dbfe43027911acdd270610048eed74a Mon Sep 17 00:00:00 2001
From: Dean Lee <deanlee3@gmail.com>
Date: Wed, 16 Jun 2021 17:01:13 +0800
Subject: [PATCH] common/util.cc: add unit test, fix bug in util::read_file
 (#21251)

* add test case for util

* fix bug in read_file

* gitignore test_util

* better test

* referer

* more

* more test

* REQUIRE size

* apply reviews

* only test binary

* simplify the test

* read  non-permission

* debug print

* remove /etc/shadow

* test read directory

* Update selfdrive/common/tests/test_util.cc

Co-authored-by: Adeeb Shihadeh <adeebshihadeh@gmail.com>
Co-authored-by: Willem Melching <willem.melching@gmail.com>
---
 .github/workflows/selfdrive_tests.yaml |  1 +
 selfdrive/common/SConscript            |  3 ++
 selfdrive/common/tests/.gitignore      |  1 +
 selfdrive/common/tests/test_util.cc    | 49 ++++++++++++++++++++++++++
 selfdrive/common/util.cc               |  4 +--
 5 files changed, 56 insertions(+), 2 deletions(-)
 create mode 100644 selfdrive/common/tests/.gitignore
 create mode 100644 selfdrive/common/tests/test_util.cc

diff --git a/.github/workflows/selfdrive_tests.yaml b/.github/workflows/selfdrive_tests.yaml
index 305d2c7b8..c12aedcce 100644
--- a/.github/workflows/selfdrive_tests.yaml
+++ b/.github/workflows/selfdrive_tests.yaml
@@ -208,6 +208,7 @@ jobs:
                         $UNIT_TEST selfdrive/athena && \
                         $UNIT_TEST selfdrive/thermald && \
                         $UNIT_TEST tools/lib/tests && \
+                        ./selfdrive/common/tests/test_util && \
                         ./selfdrive/camerad/test/ae_gray_test"
     - name: Upload coverage to Codecov
       run: bash <(curl -s https://codecov.io/bash) -v -F unit_tests
diff --git a/selfdrive/common/SConscript b/selfdrive/common/SConscript
index 8f6c1dc18..57c789479 100644
--- a/selfdrive/common/SConscript
+++ b/selfdrive/common/SConscript
@@ -35,3 +35,6 @@ else:
 
 _gpucommon = fxn('gpucommon', files, LIBS=_gpu_libs)
 Export('_common', '_gpucommon', '_gpu_libs')
+
+if GetOption('test'):
+  env.Program('tests/test_util', ['tests/test_util.cc'], LIBS=[_common])
diff --git a/selfdrive/common/tests/.gitignore b/selfdrive/common/tests/.gitignore
new file mode 100644
index 000000000..2b7a3c6eb
--- /dev/null
+++ b/selfdrive/common/tests/.gitignore
@@ -0,0 +1 @@
+test_util
diff --git a/selfdrive/common/tests/test_util.cc b/selfdrive/common/tests/test_util.cc
new file mode 100644
index 000000000..fbf190576
--- /dev/null
+++ b/selfdrive/common/tests/test_util.cc
@@ -0,0 +1,49 @@
+
+#include <dirent.h>
+#include <sys/types.h>
+
+#include <algorithm>
+#include <climits>
+#include <random>
+#include <string>
+
+#define CATCH_CONFIG_MAIN
+#include "catch2/catch.hpp"
+#include "selfdrive/common/util.h"
+
+std::string random_bytes(int size) {
+  std::random_device rd;
+  std::independent_bits_engine<std::default_random_engine, CHAR_BIT, unsigned char> rbe(rd());
+  std::string bytes(size+1, '\0');
+  std::generate(bytes.begin(), bytes.end(), std::ref(rbe));
+  return bytes;
+}
+
+TEST_CASE("util::read_file") {
+  SECTION("read /proc") {
+    std::string ret = util::read_file("/proc/self/cmdline");
+    REQUIRE(ret.find("test_util") != std::string::npos);
+  }
+  SECTION("read file") {
+    char filename[] = "/tmp/test_read_XXXXXX";
+    int fd = mkstemp(filename);
+
+    REQUIRE(util::read_file(filename).empty());
+    
+    std::string content = random_bytes(64 * 1024);
+    write(fd, content.c_str(), content.size());
+    std::string ret = util::read_file(filename);
+    REQUIRE(ret == content);
+    close(fd);
+  }
+  SECTION("read directory") {
+    REQUIRE(util::read_file(".").empty());
+  }
+  SECTION("read non-existent file") {
+    std::string ret = util::read_file("does_not_exist");
+    REQUIRE(ret.empty());
+  }
+  SECTION("read non-permission") {
+    REQUIRE(util::read_file("/proc/kmsg").empty());
+  }
+}
diff --git a/selfdrive/common/util.cc b/selfdrive/common/util.cc
index ce6fa7d80..af87d9c22 100644
--- a/selfdrive/common/util.cc
+++ b/selfdrive/common/util.cc
@@ -56,8 +56,8 @@ namespace util {
 std::string read_file(const std::string& fn) {
   std::ifstream ifs(fn, std::ios::binary | std::ios::ate);
   if (ifs) {
-    std::ifstream::pos_type pos = ifs.tellg();
-    if (pos != std::ios::beg) {
+    int pos = ifs.tellg();
+    if (pos > 0) {
       std::string result;
       result.resize(pos);
       ifs.seekg(0, std::ios::beg);