From 6a2d86c5264642b16a018d6b4047cda62b62dca9 Mon Sep 17 00:00:00 2001
From: Florent Rougon <f.rougon@free.fr>
Date: Thu, 26 Jan 2017 09:38:41 +0100
Subject: [PATCH] Small improvements for simgear::Dir

- Add method simgear::Dir::isNull(), analogous to SGPath::isNull().

- Make sure that simgear::Dir::tempDir() returns a null simgear::Dir
  instance when creation of the directory failed (so far, this was only
  the case on systems where HAVE_MKDTEMP is defined).

- Use simgear::strutils::error_string() instead of strerror() (the
  latter is not guaranteed to be thread-safe).

- Make sure <cerrno> is #included, regardless of the platform.

- Add automated tests for isNull(), setRemoveOnDestroy() and tempDir().
---
 simgear/misc/CMakeLists.txt  |  4 ++++
 simgear/misc/sg_dir.cxx      | 21 +++++++++++++-----
 simgear/misc/sg_dir.hxx      | 12 +++++++++--
 simgear/misc/sg_dir_test.cxx | 42 ++++++++++++++++++++++++++++++++++++
 4 files changed, 72 insertions(+), 7 deletions(-)
 create mode 100644 simgear/misc/sg_dir_test.cxx

diff --git a/simgear/misc/CMakeLists.txt b/simgear/misc/CMakeLists.txt
index 162f11ec..4a4ef3d3 100644
--- a/simgear/misc/CMakeLists.txt
+++ b/simgear/misc/CMakeLists.txt
@@ -71,6 +71,10 @@ add_executable(test_path path_test.cxx )
 add_test(path ${EXECUTABLE_OUTPUT_PATH}/test_path)
 target_link_libraries(test_path ${TEST_LIBS})
 
+add_executable(test_sg_dir sg_dir_test.cxx)
+target_link_libraries(test_sg_dir ${TEST_LIBS})
+add_test(sg_dir ${EXECUTABLE_OUTPUT_PATH}/test_sg_dir)
+
 endif(ENABLE_TESTS)
 
 add_boost_test(SimpleMarkdown
diff --git a/simgear/misc/sg_dir.cxx b/simgear/misc/sg_dir.cxx
index 0c8065c3..79a6ff97 100644
--- a/simgear/misc/sg_dir.cxx
+++ b/simgear/misc/sg_dir.cxx
@@ -37,7 +37,6 @@
 #  include <dirent.h>
 #  include <sys/stat.h>
 #  include <unistd.h>
-#  include <errno.h>
 #endif
 
 #include <simgear/misc/strutils.hxx>
@@ -46,6 +45,7 @@
 
 #include <cstring>
 #include <cstdlib>
+#include <cerrno>
 #include <iostream>
 #include <algorithm> // for std::sort
 
@@ -94,7 +94,7 @@ Dir Dir::current()
 #endif
     if (!buf) {
         if  (errno == 2) throw sg_exception("The current directory is invalid");
-        else throw sg_exception(strerror(errno));
+        else throw sg_exception(simgear::strutils::error_string(errno));
     }
 
     SGPath p(buf);
@@ -118,7 +118,8 @@ Dir Dir::tempDir(const std::string& templ)
     std::string s = p.local8BitStr();
     ::snprintf(buf, 1024, "%s", s.c_str());
     if (!mkdtemp(buf)) {
-        SG_LOG(SG_IO, SG_WARN, "mkdtemp failed:" << strerror(errno));
+        SG_LOG(SG_IO, SG_WARN,
+               "mkdtemp failed: " << simgear::strutils::error_string(errno));
         return Dir();
     }
     
@@ -135,6 +136,7 @@ Dir Dir::tempDir(const std::string& templ)
     Dir t(p);
     if (!t.create(0700)) {
         SG_LOG(SG_IO, SG_WARN, "failed to create temporary directory at " << p);
+        return Dir();
     }
     
     return t;
@@ -277,6 +279,11 @@ PathList Dir::children(int types, const std::string& nameFilter) const
   return result;
 }
 
+bool Dir::isNull() const
+{
+  return _path.isNull();
+}
+
 bool Dir::isEmpty() const
 {
 #if defined(SG_WINDOWS)
@@ -333,7 +340,9 @@ bool Dir::create(mode_t mode)
     int err = mkdir(ps.c_str(), mode);
 #endif
     if (err) {
-        SG_LOG(SG_IO, SG_WARN,  "directory creation failed: (" << _path << ") " << strerror(errno) );
+        SG_LOG(SG_IO, SG_WARN,
+               "directory creation failed for '" << _path.utf8Str() << "': " <<
+               simgear::strutils::error_string(errno));
     }
     
     return (err == 0);
@@ -386,7 +395,9 @@ bool Dir::remove(bool recursive)
     int err = rmdir(ps.c_str());
 #endif
     if (err) {
-        SG_LOG(SG_IO, SG_WARN, "rmdir failed:" << _path << ":" << strerror(errno));
+        SG_LOG(SG_IO, SG_WARN,
+               "rmdir failed for '" << _path.utf8Str() << "': " <<
+               simgear::strutils::error_string(errno));
     }
     return (err == 0);
 }
diff --git a/simgear/misc/sg_dir.hxx b/simgear/misc/sg_dir.hxx
index 336c2ee1..a3725e0d 100644
--- a/simgear/misc/sg_dir.hxx
+++ b/simgear/misc/sg_dir.hxx
@@ -55,7 +55,8 @@ namespace simgear
     static Dir current();
     
     /**
-     * create a temporary directory, using the supplied name
+     * Create a temporary directory, using the supplied name.
+     * The return value 'd' is such that d.isNull() in case this failed.
      */
     static Dir tempDir(const std::string& templ);
       
@@ -71,7 +72,14 @@ namespace simgear
     };
     
     PathList children(int types = 0, const std::string& nameGlob = "") const;
-    
+
+    /**
+     * Check if the underlying SGPath is null.
+     *
+     * Note: this is the case for a default-constructed Dir instance.
+     */
+    bool isNull() const;
+
     /**
      * test if the directory contains no children (except '.' and '..')
      */
diff --git a/simgear/misc/sg_dir_test.cxx b/simgear/misc/sg_dir_test.cxx
new file mode 100644
index 00000000..025f4d89
--- /dev/null
+++ b/simgear/misc/sg_dir_test.cxx
@@ -0,0 +1,42 @@
+#include <cstdlib>
+
+#include <simgear/misc/sg_path.hxx>
+#include <simgear/misc/test_macros.hxx>
+#include "sg_dir.hxx"
+
+
+void test_isNull()
+{
+    SG_VERIFY(simgear::Dir().isNull());
+}
+
+void test_setRemoveOnDestroy()
+{
+    SGPath path;
+    {
+        simgear::Dir d = simgear::Dir::tempDir("FlightGear");
+        SG_VERIFY(!d.isNull() && d.exists() && d.isEmpty());
+        d.setRemoveOnDestroy();
+
+        path = d.path();        // keep a copy of the path
+        SG_VERIFY(path.exists() && path.isDir());
+    }
+
+    SG_VERIFY(!path.exists());
+}
+
+void test_tempDir()
+{
+    simgear::Dir d = simgear::Dir::tempDir("FlightGear");
+    SG_VERIFY(!d.isNull() && d.exists() && d.isEmpty());
+    d.remove();
+}
+
+int main(int argc, char **argv)
+{
+    test_isNull();
+    test_setRemoveOnDestroy();
+    test_tempDir();
+
+    return EXIT_SUCCESS;
+}