engine: use thread-safe localtime_r(), check results

Klocwork complained about not checking the result. Depending on the
situation we now give up or use some kind of fallback (for example,
when logging).

SyncEvolution itself is not multithreaded, but utility libraries
might be, so better avoid the localtime() function and its global
state. May also be relevant when function A gets a tm * pointer,
calls function B which calls localtime() again, then A tries to
use its original value.

3 files changed, 27 insertions, 7 deletions

diff --git a/src/syncevo/SyncContext.cpp b/src/syncevo/SyncContext.cpp
index b475ccc1..5a1c5b22 100644
--- a/src/syncevo/SyncContext.cpp
+++ b/src/syncevo/SyncContext.cpp
@@ -445,7 +445,11 @@ public:
             if (mode == SESSION_CREATE) {
                 // create unique directory name in the given directory
                 time_t ts = time(NULL);
-                struct tm *tm = localtime(&ts);
+                struct tm tmbuffer;
+                struct tm *tm = localtime_r(&ts, &tmbuffer);
+                if (!tm) {
+                    SE_THROW("localtime_r() failed");
+                }
                 stringstream base;
                 base << "-"
                      << setfill('0')
@@ -912,9 +916,16 @@ private:
     void writeTimestamp(const string &key, time_t val, bool flush = true) {
         if (m_info) {
             char buffer[160];
-            struct tm tm;
+            struct tm tmbuffer, *tm;
             // be nice and store a human-readable date in addition the seconds since the epoch
-            strftime(buffer, sizeof(buffer), "%s, %Y-%m-%d %H:%M:%S %z", localtime_r(&val, &tm));
+            tm = localtime_r(&val, &tmbuffer);
+            if (tm) {
+                strftime(buffer, sizeof(buffer), "%s, %Y-%m-%d %H:%M:%S %z", tm);
+            } else {
+                // Less suitable fallback. Won't work correctly for 32
+                // bit long beyond 2038.
+                sprintf(buffer, "%lu", (long unsigned)val);
+            }
             m_info->setProperty(key, buffer);
             if (flush) {
                 m_info->flush();
diff --git a/src/syncevo/SyncML.cpp b/src/syncevo/SyncML.cpp
index d2da2e3a..5afef8d9 100644
--- a/src/syncevo/SyncML.cpp
+++ b/src/syncevo/SyncML.cpp
@@ -967,9 +967,14 @@ std::string SyncReport::formatSyncTimes() const
     if (!m_start) {
         out << "unknown";
     } else {
-        char buffer[160];
-        strftime(buffer, sizeof(buffer), "%c", localtime(&m_start));
-        out << buffer;
+        struct tm tmbuffer, *tm = localtime_r(&m_start, &tmbuffer);
+        if (tm) {
+            char buffer[160];
+            strftime(buffer, sizeof(buffer), "%c", tm);
+            out << buffer;
+        } else {
+            out << "???";
+        }
         if (!m_end) {
             out << ", unknown duration (crashed?!)";
         } else {
diff --git a/src/syncevo/util.cpp b/src/syncevo/util.cpp
index 783142dc..76524a2b 100644
--- a/src/syncevo/util.cpp
+++ b/src/syncevo/util.cpp
@@ -1039,7 +1039,11 @@ ScopedEnvChange::~ScopedEnvChange()
 std::string getCurrentTime()
 {
     time_t seconds = time (NULL);
-    tm *data = localtime (&seconds);
+    tm tmbuffer;
+    tm *data = localtime_r(&seconds, &tmbuffer);
+    if (!data) {
+        return "???";
+    }
     arrayptr<char> buffer (new char [13]);
     strftime (buffer.get(), 13, "%y%m%d%H%M%S", data);
     return buffer.get();