Long live qEnvironmentVariable()

[ChangeLog][QtCore] Added qEnvironmentVariable, which returns the value
of an environment variable in a QString, while qgetenv continues to be
used to return it in a QByteArray. For Unix, since most environment
variables seem to contain path names, qEnvironmentVariable will do the
same as QFile::decodeName, which means NFC/NFD conversion on Apple OSes.

I opted not to #include <qfile.h> from qglobal.cpp to implement that
QFile::decodeName functionality, so qglobal.cpp doesn't depend on
corelib/io and to avoid possible recursions.

Task-number: QTBUG-41006
Change-Id: I14839ba5678944c2864bffff141794b8aaa7aa28
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

src/corelib/global/qglobal.cpp

@@ -3152,24 +3152,27 @@ void *qMemSet(void *dest, int c, size_t n) { return memset(dest, c, n); }
 // add thread-safety for the Qt wrappers.
 static QBasicMutex environmentMutex;
 
-// getenv is declared as deprecated in VS2005. This function
-// makes use of the new secure getenv function.
 /*!
     \relates <QtGlobal>
+    \threadsafe
+
+    Returns the value of the environment variable with name \a varName as a
+    QByteArray. If no variable by that name is found in the environment, this
+    function returns a default-constructed QByteArray.
+
+    The Qt environment manipulation functions are thread-safe, but this
+    requires that the C library equivalent functions like getenv and putenv are
+    not directly called.
 
-    Returns the value of the environment variable with name \a
-    varName. To get the variable string, use QByteArray::constData().
     To convert the data to a QString use QString::fromLocal8Bit().
 
-    \note qgetenv() was introduced because getenv() from the standard
+    \note on desktop Windows, qgetenv() may produce data loss if the
-    C library was deprecated in VC2005 (and later versions). qgetenv()
+    original string contains Unicode characters not representable in the
-    uses the new replacement function in VC, and calls the standard C
+    ANSI encoding. Use qEnvironmentVariable() instead.
-    library's implementation on all other platforms.
+    On Unix systems, this function is lossless.
 
-    \warning Don't use qgetenv on Windows if the content may contain
+    \sa qputenv(), qEnvironmentVariable(), qEnvironmentVariableIsSet(),
-    non-US-ASCII characters, like file paths.
+    qEnvironmentVariableIsEmpty()
-
-    \sa qputenv(), qEnvironmentVariableIsSet(), qEnvironmentVariableIsEmpty()
 */
 QByteArray qgetenv(const char *varName)
 {
@@ -3191,6 +3194,87 @@ QByteArray qgetenv(const char *varName)
 #endif
 }
 
+
+/*!
+    \relates <QtGlobal>
+    \since 5.10
+
+    Returns the value of the environment variable with name \a varName as a
+    QString. If no variable by that name is found in the environment, this
+    function returns \a defaultValue.
+
+    The Qt environment manipulation functions are thread-safe, but this
+    requires that the C library equivalent functions like getenv and putenv are
+    not directly called.
+
+    The following table describes how to choose between qgetenv() and
+    qEnvironmentVariable():
+    \table
+      \header \li Condition         \li Recommendation
+      \row
+        \li Variable contains file paths or user text
+        \li qEnvironmentVariable()
+      \row
+        \li Windows-specific code
+        \li qEnvironmentVariable()
+      \row
+        \li Unix-specific code, destination variable is not QString and/or is
+            used to interface with non-Qt APIs
+        \li qgetenv()
+      \row
+        \li Destination variable is a QString
+        \li qEnvironmentVariable()
+      \row
+        \li Destination variable is a QByteArray or std::string
+        \li qgetenv()
+    \endtable
+
+    \note on Unix systems, this function may produce data loss if the original
+    string contains arbitrary binary data that cannot be decoded by the locale
+    codec. Use qgetenv() instead for that case. On Windows, this function is
+    lossless.
+
+    \note the variable name \a varName must contain only US-ASCII characters.
+
+    \sa qputenv(), qgetenv(), qEnvironmentVariableIsSet(), qEnvironmentVariableIsEmpty()
+*/
+QString qEnvironmentVariable(const char *varName, const QString &defaultValue)
+{
+#if defined(Q_OS_WIN) && !defined(Q_OS_WINRT)
+    QMutexLocker locker(&environmentMutex);
+    QVarLengthArray<wchar_t, 32> wname(int(strlen(varName)) + 1);
+    for (int i = 0; i < wname.size(); ++i) // wname.size() is correct: will copy terminating null
+        wname[i] = uchar(varName[i]);
+    size_t requiredSize = 0;
+    QString buffer;
+    _wgetenv_s(&requiredSize, 0, 0, wname.data());
+    if (requiredSize == 0)
+        return defaultValue;
+    buffer.resize(int(requiredSize));
+    _wgetenv_s(&requiredSize, reinterpret_cast<wchar_t *>(buffer.data()), requiredSize,
+               wname.data());
+    // requiredSize includes the terminating null, which we don't want.
+    Q_ASSERT(buffer.endsWith(QLatin1Char('\0')));
+    buffer.chop(1);
+    return buffer;
+#else
+    QByteArray value = qgetenv(varName);
+    if (value.isNull())
+        return defaultValue;
+// duplicated in qfile.h (QFile::decodeName)
+#if defined(Q_OS_DARWIN)
+    return QString::fromUtf8(value).normalized(QString::NormalizationForm_C);
+#else // other Unix
+    return QString::fromLocal8Bit(value);
+#endif
+#endif
+}
+
+QString qEnvironmentVariable(const char *varName)
+{
+    return qEnvironmentVariable(varName, QString());
+}
+
 /*!
     \relates <QtGlobal>
     \since 5.1
@@ -3203,7 +3287,7 @@ QByteArray qgetenv(const char *varName)
     \endcode
     except that it's potentially much faster, and can't throw exceptions.
 
-    \sa qgetenv(), qEnvironmentVariableIsSet()
+    \sa qgetenv(), qEnvironmentVariable(), qEnvironmentVariableIsSet()
 */
 bool qEnvironmentVariableIsEmpty(const char *varName) Q_DECL_NOEXCEPT
 {
@@ -3240,7 +3324,7 @@ bool qEnvironmentVariableIsEmpty(const char *varName) Q_DECL_NOEXCEPT
     are too long will either be truncated or this function will set \a ok to \c
     false.
 
-    \sa qgetenv(), qEnvironmentVariableIsSet()
+    \sa qgetenv(), qEnvironmentVariable(), qEnvironmentVariableIsSet()
 */
 int qEnvironmentVariableIntValue(const char *varName, bool *ok) Q_DECL_NOEXCEPT
 {
@@ -3291,7 +3375,7 @@ int qEnvironmentVariableIntValue(const char *varName, bool *ok) Q_DECL_NOEXCEPT
     \endcode
     except that it's potentially much faster, and can't throw exceptions.
 
-    \sa qgetenv(), qEnvironmentVariableIsEmpty()
+    \sa qgetenv(), qEnvironmentVariable(), qEnvironmentVariableIsEmpty()
 */
 bool qEnvironmentVariableIsSet(const char *varName) Q_DECL_NOEXCEPT
 {
@@ -3321,7 +3405,7 @@ bool qEnvironmentVariableIsSet(const char *varName) Q_DECL_NOEXCEPT
     uses the replacement function in VC, and calls the standard C
     library's implementation on all other platforms.
 
-    \sa qgetenv()
+    \sa qgetenv(), qEnvironmentVariable()
 */
 bool qputenv(const char *varName, const QByteArray& value)
 {
@@ -3352,7 +3436,7 @@ bool qputenv(const char *varName, const QByteArray& value)
 
     \since 5.1
 
-    \sa qputenv(), qgetenv()
+    \sa qputenv(), qgetenv(), qEnvironmentVariable()
 */
 bool qunsetenv(const char *varName)
 {

src/corelib/global/qglobal.h

@@ -1126,6 +1126,13 @@ template <typename... Args> Q_CONSTEXPR Q_DECL_UNUSED QNonConstOverload<Args...>
 
 class QByteArray;
 Q_CORE_EXPORT QByteArray qgetenv(const char *varName);
+#ifdef Q_QDOC
+Q_CORE_EXPORT QString qEnvironmentVariable(const char *varName,
+                                           const QString &defaultValue = QString());
+#else // need it as two functions because QString is only forward-declared here
+Q_CORE_EXPORT QString qEnvironmentVariable(const char *varName);
+Q_CORE_EXPORT QString qEnvironmentVariable(const char *varName, const QString &defaultValue);
+#endif
 Q_CORE_EXPORT bool qputenv(const char *varName, const QByteArray& value);
 Q_CORE_EXPORT bool qunsetenv(const char *varName);
 

src/corelib/io/qfile.h

@@ -81,6 +81,7 @@ public:
     }
     static QString decodeName(const QByteArray &localFileName)
     {
+        // note: duplicated in qglobal.cpp (qEnvironmentVariable)
         return QString::fromUtf8(localFileName).normalized(QString::NormalizationForm_C);
     }
 #else

tests/auto/corelib/global/qgetputenv/tst_qgetputenv.cpp

@@ -1,6 +1,7 @@
 /****************************************************************************
 **
 ** Copyright (C) 2016 The Qt Company Ltd.
+** Copyright (C) 2016 Intel Corporation.
 ** Contact: https://www.qt.io/licensing/
 **
 ** This file is part of the test suite of the Qt Toolkit.
@@ -30,12 +31,16 @@
 #include <QtTest/QtTest>
 
 #include <qglobal.h>
+#ifdef Q_OS_WIN
+#include <qt_windows.h>
+#endif
 
 class tst_QGetPutEnv : public QObject
 {
 Q_OBJECT
 private slots:
     void getSetCheck();
+    void encoding();
     void intValue_data();
     void intValue();
 };
@@ -53,7 +58,11 @@ void tst_QGetPutEnv::getSetCheck()
     QCOMPARE(qEnvironmentVariableIntValue(varName, &ok), 0);
     QVERIFY(!ok);
     QByteArray result = qgetenv(varName);
-    QCOMPARE(result, QByteArray());
+    QVERIFY(result.isNull());
+    QString sresult = qEnvironmentVariable(varName);
+    QVERIFY(sresult.isNull());
+    sresult = qEnvironmentVariable(varName, "hello");
+    QCOMPARE(sresult, QString("hello"));
 
 #ifndef Q_OS_WIN
     QVERIFY(qputenv(varName, "")); // deletes varName instead of making it empty, on Windows
@@ -64,6 +73,16 @@ void tst_QGetPutEnv::getSetCheck()
     QCOMPARE(qEnvironmentVariableIntValue(varName), 0);
     QCOMPARE(qEnvironmentVariableIntValue(varName, &ok), 0);
     QVERIFY(!ok);
+
+    result = qgetenv(varName);
+    QVERIFY(!result.isNull());
+    QCOMPARE(result, QByteArray());
+    sresult = qEnvironmentVariable(varName);
+    QVERIFY(!sresult.isNull());
+    QCOMPARE(sresult, QString());
+    sresult = qEnvironmentVariable(varName, "hello");
+    QVERIFY(!sresult.isNull());
+    QCOMPARE(sresult, QString());
 #endif
 
     QVERIFY(qputenv(varName, QByteArray("supervalue")));
@@ -76,19 +95,61 @@ void tst_QGetPutEnv::getSetCheck()
     QVERIFY(!ok);
     result = qgetenv(varName);
     QCOMPARE(result, QByteArrayLiteral("supervalue"));
+    sresult = qEnvironmentVariable(varName);
+    QCOMPARE(sresult, QString("supervalue"));
+    sresult = qEnvironmentVariable(varName, "hello");
+    QCOMPARE(sresult, QString("supervalue"));
 
     qputenv(varName,QByteArray());
 
     // Now test qunsetenv
     QVERIFY(qunsetenv(varName));
-    QVERIFY(!qEnvironmentVariableIsSet(varName));
+    QVERIFY(!qEnvironmentVariableIsSet(varName)); // note: might fail on some systems!
     QVERIFY(qEnvironmentVariableIsEmpty(varName));
     ok = true;
     QCOMPARE(qEnvironmentVariableIntValue(varName), 0);
     QCOMPARE(qEnvironmentVariableIntValue(varName, &ok), 0);
     QVERIFY(!ok);
+
     result = qgetenv(varName);
-    QCOMPARE(result, QByteArray());
+    QVERIFY(result.isNull());
+    sresult = qEnvironmentVariable(varName);
+    QVERIFY(sresult.isNull());
+    sresult = qEnvironmentVariable(varName, "hello");
+    QCOMPARE(sresult, QString("hello"));
+}
+
+void tst_QGetPutEnv::encoding()
+{
+    // The test string is:
+    //  U+0061      LATIN SMALL LETTER A
+    //  U+00E1      LATIN SMALL LETTER A WITH ACUTE
+    //  U+03B1      GREEK SMALL LETTER ALPHA
+    //  U+0430      CYRILLIC SMALL LETTER A
+    // This has letters in three different scripts, so no locale besides
+    // UTF-8 is able handle them all.
+    // The LATIN SMALL LETTER A WITH ACUTE is NFC for NFD:
+    //  U+0061 U+0301   LATIN SMALL LETTER A + COMBINING ACUTE ACCENT
+
+    const char varName[] = "should_not_exist";
+    static const wchar_t rawvalue[] = { 'a', 0x00E1, 0x03B1, 0x0430, 0 };
+    QString value = QString::fromWCharArray(rawvalue);
+
+#if defined(Q_OS_WINRT)
+    QSKIP("Test cannot be run on this platform");
+#elif defined(Q_OS_WIN)
+    const wchar_t wvarName[] = L"should_not_exist";
+    _wputenv_s(wvarName, rawvalue);
+#else
+    // confirm the locale is UTF-8
+    if (value.toLocal8Bit() != "a\xc3\xa1\xce\xb1\xd0\xb0")
+        QSKIP("Locale is not UTF-8, cannot test");
+
+    qputenv(varName, QFile::encodeName(value));
+#endif
+
+    QVERIFY(qEnvironmentVariableIsSet(varName));
+    QCOMPARE(qEnvironmentVariable(varName), value);
 }
 
 void tst_QGetPutEnv::intValue_data()