Use QMap in QProcessEnvironment so variables are sorted
The motivation for this change is to make it simple to pass a correctly sorted environment block to Win32 CreateProcess(). It is also nice in other contexts that the environment variables are sorted. The change is made for all platforms. This keeps it simple and the only ill effect is slightly slower lookups. Concerning the environment block passed to Win32 CreateProcess: The environment block that is passed to CreateProcess() must be sorted case-insensitively and without regard to locale. See https://msdn.microsoft.com/en-us/library/windows/desktop/ms682009(v=vs.85).aspx The need for sorting the environment block is also mentioned in the CreateProcess() documentation, but with less details: https://msdn.microsoft.com/en-us/library/windows/desktop/ms682425(v=vs.85).aspx Task-number: QTBUG-61315 Change-Id: Ie1edd443301de79cf5f699d45beab01b7c0f9de3 Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
This commit is contained in:
parent
b438110028
commit
65a317e674
@ -55,6 +55,7 @@
|
|||||||
#include "QtCore/qprocess.h"
|
#include "QtCore/qprocess.h"
|
||||||
#include "QtCore/qstringlist.h"
|
#include "QtCore/qstringlist.h"
|
||||||
#include "QtCore/qhash.h"
|
#include "QtCore/qhash.h"
|
||||||
|
#include "QtCore/qmap.h"
|
||||||
#include "QtCore/qshareddata.h"
|
#include "QtCore/qshareddata.h"
|
||||||
#include "private/qiodevice_p.h"
|
#include "private/qiodevice_p.h"
|
||||||
|
|
||||||
@ -90,22 +91,19 @@ public:
|
|||||||
QProcEnvKey(const QProcEnvKey &other) : QString(other) {}
|
QProcEnvKey(const QProcEnvKey &other) : QString(other) {}
|
||||||
bool operator==(const QProcEnvKey &other) const { return !compare(other, Qt::CaseInsensitive); }
|
bool operator==(const QProcEnvKey &other) const { return !compare(other, Qt::CaseInsensitive); }
|
||||||
};
|
};
|
||||||
inline uint qHash(const QProcEnvKey &key) { return qHash(key.toCaseFolded()); }
|
|
||||||
|
inline bool operator<(const QProcEnvKey &a, const QProcEnvKey &b)
|
||||||
|
{
|
||||||
|
// On windows use case-insensitive ordering because that is how Windows needs the environment
|
||||||
|
// block sorted (https://msdn.microsoft.com/en-us/library/windows/desktop/ms682009(v=vs.85).aspx)
|
||||||
|
return a.compare(b, Qt::CaseInsensitive) < 0;
|
||||||
|
}
|
||||||
|
|
||||||
|
Q_DECLARE_TYPEINFO(QProcEnvKey, Q_MOVABLE_TYPE);
|
||||||
|
|
||||||
typedef QString QProcEnvValue;
|
typedef QString QProcEnvValue;
|
||||||
#else
|
#else
|
||||||
class QProcEnvKey
|
using QProcEnvKey = QByteArray;
|
||||||
{
|
|
||||||
public:
|
|
||||||
QProcEnvKey() : hash(0) {}
|
|
||||||
explicit QProcEnvKey(const QByteArray &other) : key(other), hash(qHash(key)) {}
|
|
||||||
QProcEnvKey(const QProcEnvKey &other) { *this = other; }
|
|
||||||
bool operator==(const QProcEnvKey &other) const { return key == other.key; }
|
|
||||||
|
|
||||||
QByteArray key;
|
|
||||||
uint hash;
|
|
||||||
};
|
|
||||||
inline uint qHash(const QProcEnvKey &key) Q_DECL_NOTHROW { return key.hash; }
|
|
||||||
|
|
||||||
class QProcEnvValue
|
class QProcEnvValue
|
||||||
{
|
{
|
||||||
@ -138,7 +136,6 @@ public:
|
|||||||
};
|
};
|
||||||
Q_DECLARE_TYPEINFO(QProcEnvValue, Q_MOVABLE_TYPE);
|
Q_DECLARE_TYPEINFO(QProcEnvValue, Q_MOVABLE_TYPE);
|
||||||
#endif
|
#endif
|
||||||
Q_DECLARE_TYPEINFO(QProcEnvKey, Q_MOVABLE_TYPE);
|
|
||||||
|
|
||||||
class QProcessEnvironmentPrivate: public QSharedData
|
class QProcessEnvironmentPrivate: public QSharedData
|
||||||
{
|
{
|
||||||
@ -161,13 +158,13 @@ public:
|
|||||||
inline Key prepareName(const QString &name) const
|
inline Key prepareName(const QString &name) const
|
||||||
{
|
{
|
||||||
Key &ent = nameMap[name];
|
Key &ent = nameMap[name];
|
||||||
if (ent.key.isEmpty())
|
if (ent.isEmpty())
|
||||||
ent = Key(name.toLocal8Bit());
|
ent = name.toLocal8Bit();
|
||||||
return ent;
|
return ent;
|
||||||
}
|
}
|
||||||
inline QString nameToString(const Key &name) const
|
inline QString nameToString(const Key &name) const
|
||||||
{
|
{
|
||||||
const QString sname = QString::fromLocal8Bit(name.key);
|
const QString sname = QString::fromLocal8Bit(name);
|
||||||
nameMap[sname] = name;
|
nameMap[sname] = name;
|
||||||
return sname;
|
return sname;
|
||||||
}
|
}
|
||||||
@ -206,8 +203,8 @@ public:
|
|||||||
}
|
}
|
||||||
#endif
|
#endif
|
||||||
|
|
||||||
typedef QHash<Key, Value> Hash;
|
using Map = QMap<Key, Value>;
|
||||||
Hash vars;
|
Map vars;
|
||||||
|
|
||||||
#ifdef Q_OS_UNIX
|
#ifdef Q_OS_UNIX
|
||||||
typedef QHash<QString, Key> NameHash;
|
typedef QHash<QString, Key> NameHash;
|
||||||
|
@ -338,7 +338,7 @@ bool QProcessPrivate::openChannel(Channel &channel)
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
static char **_q_dupEnvironment(const QProcessEnvironmentPrivate::Hash &environment, int *envc)
|
static char **_q_dupEnvironment(const QProcessEnvironmentPrivate::Map &environment, int *envc)
|
||||||
{
|
{
|
||||||
*envc = 0;
|
*envc = 0;
|
||||||
if (environment.isEmpty())
|
if (environment.isEmpty())
|
||||||
@ -351,7 +351,7 @@ static char **_q_dupEnvironment(const QProcessEnvironmentPrivate::Hash &environm
|
|||||||
auto it = environment.constBegin();
|
auto it = environment.constBegin();
|
||||||
const auto end = environment.constEnd();
|
const auto end = environment.constEnd();
|
||||||
for ( ; it != end; ++it) {
|
for ( ; it != end; ++it) {
|
||||||
QByteArray key = it.key().key;
|
QByteArray key = it.key();
|
||||||
QByteArray value = it.value().bytes();
|
QByteArray value = it.value().bytes();
|
||||||
key.reserve(key.length() + 1 + value.length());
|
key.reserve(key.length() + 1 + value.length());
|
||||||
key.append('=');
|
key.append('=');
|
||||||
|
@ -390,11 +390,11 @@ static QString qt_create_commandline(const QString &program, const QStringList &
|
|||||||
return args;
|
return args;
|
||||||
}
|
}
|
||||||
|
|
||||||
static QByteArray qt_create_environment(const QProcessEnvironmentPrivate::Hash &environment)
|
static QByteArray qt_create_environment(const QProcessEnvironmentPrivate::Map &environment)
|
||||||
{
|
{
|
||||||
QByteArray envlist;
|
QByteArray envlist;
|
||||||
if (!environment.isEmpty()) {
|
if (!environment.isEmpty()) {
|
||||||
QProcessEnvironmentPrivate::Hash copy = environment;
|
QProcessEnvironmentPrivate::Map copy = environment;
|
||||||
|
|
||||||
// add PATH if necessary (for DLL loading)
|
// add PATH if necessary (for DLL loading)
|
||||||
QProcessEnvironmentPrivate::Key pathKey(QLatin1String("PATH"));
|
QProcessEnvironmentPrivate::Key pathKey(QLatin1String("PATH"));
|
||||||
|
@ -94,6 +94,7 @@ private slots:
|
|||||||
void setEnvironment();
|
void setEnvironment();
|
||||||
void setProcessEnvironment_data();
|
void setProcessEnvironment_data();
|
||||||
void setProcessEnvironment();
|
void setProcessEnvironment();
|
||||||
|
void environmentIsSorted();
|
||||||
void spaceInName();
|
void spaceInName();
|
||||||
void setStandardInputFile();
|
void setStandardInputFile();
|
||||||
void setStandardOutputFile_data();
|
void setStandardOutputFile_data();
|
||||||
@ -1733,6 +1734,47 @@ void tst_QProcess::setProcessEnvironment()
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void tst_QProcess::environmentIsSorted()
|
||||||
|
{
|
||||||
|
QProcessEnvironment env;
|
||||||
|
env.insert(QLatin1String("a"), QLatin1String("foo_a"));
|
||||||
|
env.insert(QLatin1String("B"), QLatin1String("foo_B"));
|
||||||
|
env.insert(QLatin1String("c"), QLatin1String("foo_c"));
|
||||||
|
env.insert(QLatin1String("D"), QLatin1String("foo_D"));
|
||||||
|
env.insert(QLatin1String("e"), QLatin1String("foo_e"));
|
||||||
|
env.insert(QLatin1String("F"), QLatin1String("foo_F"));
|
||||||
|
env.insert(QLatin1String("Path"), QLatin1String("foo_Path"));
|
||||||
|
env.insert(QLatin1String("SystemRoot"), QLatin1String("foo_SystemRoot"));
|
||||||
|
|
||||||
|
const QStringList envlist = env.toStringList();
|
||||||
|
|
||||||
|
#ifdef Q_OS_WIN32
|
||||||
|
// The environment block passed to CreateProcess "[Requires that] All strings in the
|
||||||
|
// environment block must be sorted alphabetically by name. The sort is case-insensitive,
|
||||||
|
// Unicode order, without regard to locale."
|
||||||
|
// https://msdn.microsoft.com/en-us/library/windows/desktop/ms682009(v=vs.85).aspx
|
||||||
|
// So on Windows we sort that way.
|
||||||
|
const QStringList expected = { QLatin1String("a=foo_a"),
|
||||||
|
QLatin1String("B=foo_B"),
|
||||||
|
QLatin1String("c=foo_c"),
|
||||||
|
QLatin1String("D=foo_D"),
|
||||||
|
QLatin1String("e=foo_e"),
|
||||||
|
QLatin1String("F=foo_F"),
|
||||||
|
QLatin1String("Path=foo_Path"),
|
||||||
|
QLatin1String("SystemRoot=foo_SystemRoot") };
|
||||||
|
#else
|
||||||
|
const QStringList expected = { QLatin1String("B=foo_B"),
|
||||||
|
QLatin1String("D=foo_D"),
|
||||||
|
QLatin1String("F=foo_F"),
|
||||||
|
QLatin1String("Path=foo_Path"),
|
||||||
|
QLatin1String("SystemRoot=foo_SystemRoot"),
|
||||||
|
QLatin1String("a=foo_a"),
|
||||||
|
QLatin1String("c=foo_c"),
|
||||||
|
QLatin1String("e=foo_e") };
|
||||||
|
#endif
|
||||||
|
QCOMPARE(envlist, expected);
|
||||||
|
}
|
||||||
|
|
||||||
void tst_QProcess::systemEnvironment()
|
void tst_QProcess::systemEnvironment()
|
||||||
{
|
{
|
||||||
QVERIFY(!QProcess::systemEnvironment().isEmpty());
|
QVERIFY(!QProcess::systemEnvironment().isEmpty());
|
||||||
|
Loading…
Reference in New Issue
Block a user