Replace readdir_r

.travis.yml

@@ -176,7 +176,9 @@ matrix:
            - moreutils
     - os: linux
       dist: xenial
-      env: TASK='codespell'
+      env:
+       - TASK='codespell'
+       - PATH=/opt/python/3.7.1/bin:$PATH
       addons:
         apt:
           packages:
@@ -312,7 +314,7 @@ install:
   - if [ "$TRAVIS_OS_NAME" == "linux" ]; then pip install --user numpy; fi
   - if [ "$TASK" = "coverage" ]; then pip install --user cpp-coveralls; fi
   - if [ "$TASK" = "flake8" ]; then pip install --user flake8; fi
-  - if [ "$TASK" = "codespell" ]; then pip install --user git+https://github.com/codespell-project/codespell.git; fi
+  - if [ "$TASK" = "codespell" ]; then pip3 install --user git+https://github.com/codespell-project/codespell.git; fi
   - if [ "$TASK" = "jshint" ]; then npm install -g grunt-cli; fi
 
 before_install:

AUTHORS

@@ -13,7 +13,7 @@ Contributors:
   Hakan Lindestaf, Renard plugin
   Harry F, for the Eurolite USB Pro code
   Heikki Junnila, bug fixes for the debian packaging files
-  Jannis Achstetter, compile fixes for newer dependencies
+  Jannis Achstetter, compile fixes for newer dependencies and various fixes
   Laurent (Renzo), Debian packages, FreeBSD & RDM testing.
   Lukas Erlinghagen, win32 port.
   Johan Nilsson, Philips Hue trigger config

NEWS

@@ -38,7 +38,8 @@ x/y/2019 ola-0.10.8
    the Python RDM Test code, also the relevant comments
 
  Internal:
- *
+ * Replace "readdir_r" with "readdir" since the former has been deprecated
+ * Add a unit test for the functions that used readdir_r before
 
 13/7/2018 ola-0.10.7
  Features:

common/file/Util.cpp

@@ -129,15 +129,19 @@ bool FindMatchingFiles(const string &directory,
   FindClose(h_find);
 #else
   DIR *dp;
-  struct dirent dir_ent;
   struct dirent *dir_ent_p;
   if ((dp = opendir(directory.data())) == NULL) {
     OLA_WARN << "Could not open " << directory << ": " << strerror(errno);
     return false;
   }
 
-  if (readdir_r(dp, &dir_ent, &dir_ent_p)) {
-    OLA_WARN << "readdir_r(" << directory << "): " << strerror(errno);
+  // Explicitly set errno to 0 so we can reliably check the value after
+  // the call to readdir. It might have a undefined value otherwise since
+  // a successful call may but doesn't need to set a value.
+  errno = 0;
+  dir_ent_p = readdir(dp);
+  if ((dir_ent_p == NULL) && (errno != 0)) {
+    OLA_WARN << "readdir(" << directory << "): " << strerror(errno);
     closedir(dp);
     return false;
   }
@@ -151,8 +155,12 @@ bool FindMatchingFiles(const string &directory,
         files->push_back(str.str());
       }
     }
-    if (readdir_r(dp, &dir_ent, &dir_ent_p)) {
-      OLA_WARN << "readdir_r(" << directory << "): " << strerror(errno);
+    // Explicitly set errno to 0 so we can reliably check the value after
+    // the call. It might have an undefined value otherwise.
+    errno = 0;
+    dir_ent_p = readdir(dp);
+    if ((dir_ent_p == NULL) && (errno != 0)) {
+      OLA_WARN << "readdir(" << directory << "): " << strerror(errno);
       closedir(dp);
       return false;
     }

common/file/UtilTest.cpp

@@ -20,26 +20,31 @@
 
 #include <cppunit/extensions/HelperMacros.h>
 #include <string>
+#include <iterator>
+#include <vector>
 
+#include "ola/StringUtils.h"
 #include "ola/file/Util.h"
 #include "ola/testing/TestUtils.h"
 
-
 using ola::file::FilenameFromPath;
 using ola::file::FilenameFromPathOrDefault;
 using ola::file::FilenameFromPathOrPath;
 using ola::file::JoinPaths;
+using ola::file::FindMatchingFiles;
 using std::string;
 
 class UtilTest: public CppUnit::TestFixture {
   CPPUNIT_TEST_SUITE(UtilTest);
   CPPUNIT_TEST(testJoinPaths);
   CPPUNIT_TEST(testFilenameFromPath);
+  CPPUNIT_TEST(testFindMatchingFiles);
   CPPUNIT_TEST_SUITE_END();
 
  public:
   void testFilenameFromPath();
   void testJoinPaths();
+  void testFindMatchingFiles();
 };
 
 
@@ -90,3 +95,57 @@ void UtilTest::testFilenameFromPath() {
   OLA_ASSERT_EQ(string("baz"), FilenameFromPathOrPath("/foo/bar/baz"));
 }
 
+/*
+ * Test the FindMatchingFiles function
+ */
+void UtilTest::testFindMatchingFiles() {
+  bool okay = false;
+
+  std::vector<std::string> files;
+  std::vector<std::string>::iterator file;
+
+  OLA_ASSERT_TRUE_MSG(ola::file::PATH_SEPARATOR == '/' ||
+                      ola::file::PATH_SEPARATOR == '\\',
+                      "ola::file::PATH_SEPARATOR is neither / nor \\");
+
+  okay = FindMatchingFiles(std::string(TEST_SRC_DIR) +
+                           ola::file::PATH_SEPARATOR + std::string("man"),
+                           std::string("rdm_"), &files);
+
+  OLA_ASSERT_TRUE_MSG(okay, "FindMatchingFiles returned false");
+
+  // At the time this test was written, there were 3 files in folder "man"
+  // starting with "rdm_". If this changed, please adapt the number below
+  // Or find something better to match against
+  OLA_ASSERT_EQ_MSG(3, (int)files.size(),
+                    "Not exactly 3 files man/rdm_* returned");
+
+  bool rdm_model_collector_found = false;
+  bool rdm_responder_test_found = false;
+  bool rdm_test_server_found = false;
+
+  // Iterate over all files that have been returned and check if the ones
+  // we expected are in that list
+  for (file = files.begin(); file < files.end(); file++) {
+    if (ola::StringEndsWith(*file, "rdm_model_collector.py.1")) {
+      // make sure it has not been reported as found before
+      OLA_ASSERT_FALSE(rdm_model_collector_found);
+      rdm_model_collector_found = true;
+    } else if (ola::StringEndsWith(*file, "rdm_responder_test.py.1")) {
+      // make sure it has not been reported as found before
+      OLA_ASSERT_FALSE(rdm_responder_test_found);
+      rdm_responder_test_found = true;
+    } else if (ola::StringEndsWith(*file, "rdm_test_server.py.1")) {
+      // make sure it has not been reported as found before
+      OLA_ASSERT_FALSE(rdm_test_server_found);
+      rdm_test_server_found = true;
+    }
+  }
+
+  OLA_ASSERT_TRUE_MSG(rdm_model_collector_found,
+                      "Result lacks rdm_model_collector.py.1");
+  OLA_ASSERT_TRUE_MSG(rdm_responder_test_found,
+                      "Result lacks rdm_responder_test.py.1");
+  OLA_ASSERT_TRUE_MSG(rdm_test_server_found,
+                      "Result lacks rdm_test_server.py.1");
+}

configure.ac

@@ -228,29 +228,6 @@ AC_CHECK_FUNCS([bzero gettimeofday memmove memset mkdir strdup strrchr \
                 socket strerror getifaddrs getloadavg getpwnam_r getpwuid_r \
                 getgrnam_r getgrgid_r secure_getenv])
 
-AC_MSG_CHECKING(for readdir_r deprecation)
-old_cxxflags=$CXXFLAGS
-CXXFLAGS="${CXXFLAGS} -Werror=deprecated-declarations"
-AC_CACHE_VAL(ac_cv_readdir_r_deprecation,
-  AC_COMPILE_IFELSE(
-     [AC_LANG_PROGRAM([[#include <dirent.h>
-#include <unistd.h>]],[[
-        const char* arg = "/";
-        DIR * dir;
-        struct dirent *buf, *de;
-        dir = opendir(arg);
-        readdir_r(dir,buf,&de);
-]])],
-     [ac_cv_readdir_r_deprecation=no],
-     [ac_cv_readdir_r_deprecation=yes])
-)
-CXXFLAGS=$old_cxxflags
-AC_MSG_RESULT($ac_cv_readdir_r_deprecation)
-
-AS_IF([test "x$ac_cv_readdir_r_deprecation" = xyes],
-      [CXXFLAGS="$CXXFLAGS -Wno-error=deprecated-declarations"],
-      [])
-
 LT_INIT([win32-dll])
 
 # Decide if we're building on Windows early on.

include/ola/Logging.h

@@ -73,7 +73,7 @@
 #define OLA_WARN OLA_LOG(ola::OLA_LOG_WARN)
 
 /**
- * Provide a stream to log an infomational message.
+ * Provide a stream to log an informational message.
  * @code
  *     OLA_INFO << "Reading configs from " << config_dir;
  * @endcode