diff --git a/src/corelib/CMakeLists.txt b/src/corelib/CMakeLists.txt index f4682e2e99..4669b6fde9 100644 --- a/src/corelib/CMakeLists.txt +++ b/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 diff --git a/src/corelib/global/qglobal_p.h b/src/corelib/global/qglobal_p.h index 258cdccf47..726e25b901 100644 --- a/src/corelib/global/qglobal_p.h +++ b/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 // until qtdeclarative's tst_qqmlqt.cpp catches up #ifndef QT_BOOTSTRAPPED #include @@ -52,17 +53,8 @@ #endif #if defined(__cplusplus) -#ifdef Q_CC_MINGW -# include // Define _POSIX_THREAD_SAFE_FUNCTIONS to obtain localtime_r() -#endif -#include - 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 - diff --git a/src/corelib/global/qtenvironmentvariables.cpp b/src/corelib/global/qtenvironmentvariables.cpp index 4046beae1f..4f2a263cc4 100644 --- a/src/corelib/global/qtenvironmentvariables.cpp +++ b/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 #include @@ -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 diff --git a/src/corelib/global/qtenvironmentvariables_p.h b/src/corelib/global/qtenvironmentvariables_p.h new file mode 100644 index 0000000000..afc3c5f52a --- /dev/null +++ b/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 // Define _POSIX_THREAD_SAFE_FUNCTIONS to obtain localtime_r() +#endif +#include + +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 diff --git a/src/corelib/time/qlocaltime.cpp b/src/corelib/time/qlocaltime.cpp index bace65841a..4f7496834a 100644 --- a/src/corelib/time/qlocaltime.cpp +++ b/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; diff --git a/tests/auto/corelib/time/qdatetime/tst_qdatetime.cpp b/tests/auto/corelib/time/qdatetime/tst_qdatetime.cpp index 25bed5170f..0b165f791a 100644 --- a/tests/auto/corelib/time/qdatetime/tst_qdatetime.cpp +++ b/tests/auto/corelib/time/qdatetime/tst_qdatetime.cpp @@ -7,8 +7,8 @@ #include #include +#include // for qTzSet() -#include #ifdef Q_OS_WIN # include #endif