Move qtLocalTime() to qtenvironmentvariables.cpp as qLocalTime()

The new name better matches the names of existing functions there.
A comment on the old code noted that such a move should really be
done, so as to correctly share the environment-mutex locking.

In the process, move the (now three) time-related internal functions
from qglobal_p.h to a new qtenvironmentvariables_p.h since that's the
natural place for them given where they're defined (and the fact that
they're for internal use only).

Change-Id: Ib028baebaf31a806a2c0c97caaaba0a466c11cea
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

src/corelib/CMakeLists.txt

@@ -89,6 +89,7 @@ qt_internal_add_module(Core
         global/qtconfigmacros.h
         global/qtdeprecationmarkers.h
         global/qtenvironmentvariables.cpp global/qtenvironmentvariables.h
+        global/qtenvironmentvariables_p.h
         global/qtnoop.h
         global/qtpreprocessorsupport.h
         global/qtrace_p.h

src/corelib/global/qglobal_p.h

@@ -18,6 +18,7 @@
 
 #include "qglobal.h"
 #include "qglobal_p.h"      // include self to avoid syncqt warning - no-op
+#include <private/qtenvironmentvariables_p.h> // until qtdeclarative's tst_qqmlqt.cpp catches up
 
 #ifndef QT_BOOTSTRAPPED
 #include <QtCore/private/qconfig_p.h>
@@ -52,17 +53,8 @@
 #endif
 
 #if defined(__cplusplus)
-#ifdef Q_CC_MINGW
-#  include <unistd.h> // Define _POSIX_THREAD_SAFE_FUNCTIONS to obtain localtime_r()
-#endif
-#include <time.h>
-
 QT_BEGIN_NAMESPACE
 
-// These behave as if they consult the environment, so need to share its locking:
-Q_CORE_EXPORT void qTzSet();
-Q_CORE_EXPORT time_t qMkTime(struct tm *when);
-
 #if !defined(Q_CC_MSVC) || defined(Q_CC_CLANG)
 Q_NORETURN
 #endif
@@ -142,4 +134,3 @@ QT_END_NAMESPACE
 #endif // defined(__cplusplus)
 
 #endif // QGLOBAL_P_H
-

src/corelib/global/qtenvironmentvariables.cpp

@@ -2,6 +2,7 @@
 // SPDX-License-Identifier: LicenseRef-Qt-Commercial OR LGPL-3.0-only OR GPL-2.0-only OR GPL-3.0-only
 
 #include "qtenvironmentvariables.h"
+#include "qtenvironmentvariables_p.h"
 
 #include <qplatformdefs.h>
 #include <QtCore/qbytearray.h>
@@ -330,9 +331,11 @@ bool qunsetenv(const char *varName)
 #endif
 }
 
-/*
-    Wraps tzset(), which accesses the environment, so should only be called while
-    we hold the lock on the environment mutex.
+/* Various time-related APIs that need to consult system settings also need
+   protection with the same lock as the environment, since those system settings
+   include part of the environment (principally TZ).
+
+   First, tzset(), which POSIX explicitly says accesses the environment.
 */
 void qTzSet()
 {
@@ -344,9 +347,8 @@ void qTzSet()
 #endif // Q_OS_WIN
 }
 
-/*
-    Wrap mktime(), which is specified to behave as if it called tzset(), hence
-    shares its implicit environment-dependence.
+/* Wrap mktime(), which is specified to behave as if it called tzset(), hence
+   shares its implicit environment-dependence.
 */
 time_t qMkTime(struct tm *when)
 {
@@ -354,4 +356,40 @@ time_t qMkTime(struct tm *when)
     return mktime(when);
 }
 
+/* For localtime(), POSIX mandates that it behave as if it called tzset().
+   For the alternatives to it, we need (if only for compatibility) to do the
+   same explicitly, which should ensure a re-parse of timezone info.
+*/
+bool qLocalTime(time_t utc, struct tm *local)
+{
+    const auto locker = qt_scoped_lock(environmentMutex);
+#if defined(Q_OS_WIN)
+    // The doc of localtime_s() says that it corrects for the same things
+    // _tzset() sets the globals for, but QTBUG-109974 reveals a need for an
+    // explicit call, all the same.
+    _tzset();
+    return !localtime_s(local, &utc);
+#elif QT_CONFIG(thread) && defined(_POSIX_THREAD_SAFE_FUNCTIONS)
+    // Use the reentrant version of localtime() where available, as it is
+    // thread-safe and doesn't use a shared static data area.
+    // As localtime_r() is not specified to work as if it called tzset(),
+    // make an explicit call.
+    tzset();
+    if (tm *res = localtime_r(&utc, local)) {
+        Q_ASSERT(res == local);
+        return true;
+    }
+    return false;
+#else
+    // POSIX mandates that localtime() behaves as if it called tzset().
+    // Returns shared static data which may be overwritten at any time (albeit
+    // our lock probably keeps it safe). So copy the result promptly:
+    if (tm *res = localtime(&utc)) {
+        *local = *res;
+        return true;
+    }
+    return false;
+#endif
+}
+
 QT_END_NAMESPACE

src/corelib/global/qtenvironmentvariables_p.h

@@ -0,0 +1,38 @@
+// Copyright (C) 2017 The Qt Company Ltd.
+// Copyright (C) 2015 Intel Corporation.
+// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR LGPL-3.0-only OR GPL-2.0-only OR GPL-3.0-only
+
+#ifndef QTENVIRONMENTVARIABLES_P_H
+#define QTENVIRONMENTVARIABLES_P_H
+// Nothing but (tests and) ../time/qlocaltime.cpp should access this.
+#if defined(__cplusplus)
+
+//
+//  W A R N I N G
+//  -------------
+//
+// This file is not part of the Qt API. It exists purely as an implementation
+// detail. This header file may change from version to version without notice,
+// or even be removed.
+//
+// We mean it.
+//
+
+#include "qglobal_p.h"
+
+#ifdef Q_CC_MINGW
+#  include <unistd.h> // Define _POSIX_THREAD_SAFE_FUNCTIONS to obtain localtime_r()
+#endif
+#include <time.h>
+
+QT_BEGIN_NAMESPACE
+
+// These (behave as if they) consult the environment, so need to share its locking:
+Q_CORE_EXPORT void qTzSet();
+Q_CORE_EXPORT time_t qMkTime(struct tm *when);
+Q_CORE_EXPORT bool qLocalTime(time_t utc, struct tm *local);
+
+QT_END_NAMESPACE
+
+#endif // defined(__cplusplus)
+#endif // QTENVIRONMENTVARIABLES_P_H

src/corelib/time/qlocaltime.cpp

@@ -10,6 +10,7 @@
 #endif
 #include "private/qgregoriancalendar_p.h"
 #include "private/qnumeric_p.h"
+#include "private/qtenvironmentvariables_p.h"
 #if QT_CONFIG(timezone)
 #include "private/qtimezoneprivate_p.h"
 #endif
@@ -164,42 +165,6 @@ struct tm timeToTm(qint64 localDay, int secs, QDateTimePrivate::DaylightStatus d
     return local;
 }
 
-bool qtLocalTime(time_t utc, struct tm *local)
-{
-    // This should really be done under the environmentMutex wrapper qglobal.cpp
-    // uses in qTzSet() and friends. However, the only sane way to do that would
-    // be to move this whole function there (and replace its qTzSet() with a
-    // naked tzset(), since it'd already be mutex-protected).
-#if defined(Q_OS_WIN)
-    // The doc of localtime_s() says that localtime_s() corrects for the same
-    // things _tzset() sets the globals for, but doesn't explicitly say that it
-    // calls _tzset(), and QTBUG-109974 reveals the need for a _tzset() call.
-    qTzSet();
-    return !localtime_s(local, &utc);
-#elif QT_CONFIG(thread) && defined(_POSIX_THREAD_SAFE_FUNCTIONS)
-    // Use the reentrant version of localtime() where available, as it is
-    // thread-safe and doesn't use a shared static data area.
-    // As localtime() is specified to work as if it called tzset(), but
-    // localtime_r() does not have this constraint, make an explicit call.
-    // The explicit call should also request a re-parse of timezone info.
-    qTzSet();
-    if (tm *res = localtime_r(&utc, local)) {
-        Q_ASSERT(res == local);
-        return true;
-    }
-    return false;
-#else
-    // POSIX mandates that localtime() behaves as if it called tzset().
-    // Returns shared static data which may be overwritten at any time
-    // So copy the result asap:
-    if (tm *res = localtime(&utc)) {
-        *local = *res;
-        return true;
-    }
-    return false;
-#endif
-}
-
 // Returns the tzname, assume tzset has been called already
 QString qt_tzname(QDateTimePrivate::DaylightStatus daylightStatus)
 {
@@ -322,7 +287,7 @@ QDateTimePrivate::ZoneState utcToLocal(qint64 utcMillis)
         return {utcMillis};
 
     tm local;
-    if (!qtLocalTime(epochSeconds, &local))
+    if (!qLocalTime(epochSeconds, &local))
         return {utcMillis};
 
     qint64 jd;

tests/auto/corelib/time/qdatetime/tst_qdatetime.cpp

@@ -7,8 +7,8 @@
 
 #include <QTimeZone>
 #include <private/qdatetime_p.h>
+#include <private/qtenvironmentvariables_p.h> // for qTzSet()
 
-#include <time.h>
 #ifdef Q_OS_WIN
 #   include <qt_windows.h>
 #endif