From c214c000ccebda30ed867934a9d56af29cb34264 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Wed, 3 Aug 2016 16:04:22 -0700 Subject: [PATCH] qEnvironmentVariableIntValue: fix the case of a non-numeric value The documentation says that it's equivalent to qgetenv(varName).toInt() But the implementation wasn't. QByteArray::toInt() verifies that the entire string was consumed, so QByteArray("1a").toInt() == 0, but qstrtoll alone doesn't. That is, qstrtoll("1a", ...) == 1. The implementation also detected the base, a behavior I kept. Instead, I updated the documentation. Change-Id: I0031aa609e714ae983c3fffd14676ea6061a9268 Reviewed-by: Marc Mutz --- src/corelib/global/qglobal.cpp | 21 ++++++++++----- .../global/qgetputenv/tst_qgetputenv.cpp | 26 ++++++++++++++++++- 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/src/corelib/global/qglobal.cpp b/src/corelib/global/qglobal.cpp index 22fc20d47e..14853b3687 100644 --- a/src/corelib/global/qglobal.cpp +++ b/src/corelib/global/qglobal.cpp @@ -3259,20 +3259,26 @@ bool qEnvironmentVariableIsEmpty(const char *varName) Q_DECL_NOEXCEPT Equivalent to \code - qgetenv(varName).toInt() + qgetenv(varName).toInt(ok, 0) \endcode except that it's much faster, and can't throw exceptions. + \note there's a limit on the length of the value, which is sufficient for + all valid values of int, not counting leading zeroes or spaces. Values that + are too long will either be truncated or this function will set \a ok to \c + false. + \sa qgetenv(), qEnvironmentVariableIsSet() */ int qEnvironmentVariableIntValue(const char *varName, bool *ok) Q_DECL_NOEXCEPT { - QMutexLocker locker(&environmentMutex); -#if defined(_MSC_VER) && _MSC_VER >= 1400 - // we provide a buffer that can hold any int value: static const int NumBinaryDigitsPerOctalDigit = 3; static const int MaxDigitsForOctalInt = (std::numeric_limits::digits + NumBinaryDigitsPerOctalDigit - 1) / NumBinaryDigitsPerOctalDigit; + + QMutexLocker locker(&environmentMutex); +#if defined(_MSC_VER) && _MSC_VER >= 1400 + // we provide a buffer that can hold any int value: char buffer[MaxDigitsForOctalInt + 2]; // +1 for NUL +1 for optional '-' size_t dummy; if (getenv_s(&dummy, buffer, sizeof buffer, varName) != 0) { @@ -3282,15 +3288,16 @@ int qEnvironmentVariableIntValue(const char *varName, bool *ok) Q_DECL_NOEXCEPT } #else const char * const buffer = ::getenv(varName); - if (!buffer || !*buffer) { + if (!buffer || strlen(buffer) > MaxDigitsForOctalInt + 2) { if (ok) *ok = false; return 0; } #endif bool ok_ = true; - const qlonglong value = qstrtoll(buffer, Q_NULLPTR, 0, &ok_); - if (int(value) != value) { // this is the check in QByteArray::toInt(), keep it in sync + const char *endptr; + const qlonglong value = qstrtoll(buffer, &endptr, 0, &ok_); + if (int(value) != value || *endptr != '\0') { // this is the check in QByteArray::toInt(), keep it in sync if (ok) *ok = false; return 0; diff --git a/tests/auto/corelib/global/qgetputenv/tst_qgetputenv.cpp b/tests/auto/corelib/global/qgetputenv/tst_qgetputenv.cpp index 66fc578d5f..09abb953ba 100644 --- a/tests/auto/corelib/global/qgetputenv/tst_qgetputenv.cpp +++ b/tests/auto/corelib/global/qgetputenv/tst_qgetputenv.cpp @@ -97,17 +97,33 @@ void tst_QGetPutEnv::intValue_data() QTest::addColumn("expected"); QTest::addColumn("ok"); - // most non-success cases already tested in getSetCheck() + // some repetition from what is tested in getSetCheck() + QTest::newRow("empty") << QByteArray() << 0 << false; + QTest::newRow("spaces-heading") << QByteArray(" 1") << 1 << true; + QTest::newRow("spaces-trailing") << QByteArray("1 ") << 0 << false; #define ROW(x, i, b) \ QTest::newRow(#x) << QByteArray(#x) << (i) << (b) ROW(auto, 0, false); + ROW(1auto, 0, false); ROW(0, 0, true); + ROW(+0, 0, true); ROW(1, 1, true); + ROW(+1, 1, true); + ROW(09, 0, false); ROW(010, 8, true); ROW(0x10, 16, true); + ROW(0x, 0, false); + ROW(0xg, 0, false); + ROW(0x1g, 0, false); + ROW(000000000000000000000000000000000000000000000000001, 0, false); + ROW(+000000000000000000000000000000000000000000000000001, 0, false); + ROW(000000000000000000000000000000000000000000000000001g, 0, false); + ROW(-0, 0, true); ROW(-1, -1, true); ROW(-010, -8, true); + ROW(-000000000000000000000000000000000000000000000000001, 0, false); + ROW(2147483648, 0, false); // ROW(0xffffffff, -1, true); // could be expected, but not how QByteArray::toInt() works ROW(0xffffffff, 0, false); const int bases[] = {10, 8, 16}; @@ -125,6 +141,7 @@ void tst_QGetPutEnv::intValue_data() void tst_QGetPutEnv::intValue() { + const int maxlen = (sizeof(int) * CHAR_BIT + 2) / 3; const char varName[] = "should_not_exist"; QFETCH(QByteArray, value); @@ -133,6 +150,13 @@ void tst_QGetPutEnv::intValue() bool actualOk = !ok; + // Self-test: confirm that it was like the docs said it should be + if (value.length() < maxlen) { + QCOMPARE(value.toInt(&actualOk, 0), expected); + QCOMPARE(actualOk, ok); + } + + actualOk = !ok; QVERIFY(qputenv(varName, value)); QCOMPARE(qEnvironmentVariableIntValue(varName), expected); QCOMPARE(qEnvironmentVariableIntValue(varName, &actualOk), expected);