From 608b08f4d57369378b1a101b78a46202bd17c11a Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Mon, 27 Mar 2023 17:57:13 -0600 Subject: [PATCH] IPC: make the two platformSafeKey functions return QNativeIpcKey There's no need to return QString, only to create the QNativeIpcKey elsewhere, when nothing uses this intermediary QString (those two functions aren't used in unit tests any more, since the two "IPC: add native key support" commits (2c286561bbc9e4e408d34e5bf43db8ad9acc0e84 and 3ae052d3bb5d7af0badf32575a5aa042cffd8243). Since they aren't used in the tests, we can remove the Q_AUTOTEST_EXPORT macro too and the unnecessary default argument. I'll need the ability to return QNativeIpcKey to store the original, legacy key inside of it. Pick-to: 6.6.0 Change-Id: Idd5e1bb52be047d7b4fffffd17506c05e4f61f79 Reviewed-by: Volker Hilsheimer (cherry picked from commit b5584ed2240abe3892fa5b0a23b3ad093ed9fec2) Reviewed-by: Qt Cherry-pick Bot --- src/corelib/ipc/qsharedmemory.cpp | 4 +- src/corelib/ipc/qsystemsemaphore.cpp | 4 +- src/corelib/ipc/qtipccommon.cpp | 82 +++++++++++++++------------- src/corelib/ipc/qtipccommon_p.h | 6 +- 4 files changed, 51 insertions(+), 45 deletions(-) diff --git a/src/corelib/ipc/qsharedmemory.cpp b/src/corelib/ipc/qsharedmemory.cpp index 139c1440e30..89bd4ffe0c0 100644 --- a/src/corelib/ipc/qsharedmemory.cpp +++ b/src/corelib/ipc/qsharedmemory.cpp @@ -686,12 +686,12 @@ bool QSharedMemory::isKeyTypeSupported(QNativeIpcKey::Type type) QNativeIpcKey QSharedMemory::platformSafeKey(const QString &key, QNativeIpcKey::Type type) { - return { QtIpcCommon::platformSafeKey(key, IpcType::SharedMemory, type), type }; + return QtIpcCommon::platformSafeKey(key, IpcType::SharedMemory, type); } QNativeIpcKey QSharedMemory::legacyNativeKey(const QString &key, QNativeIpcKey::Type type) { - return { legacyPlatformSafeKey(key, IpcType::SharedMemory, type), type }; + return QtIpcCommon::legacyPlatformSafeKey(key, IpcType::SharedMemory, type); } #endif // QT_CONFIG(sharedmemory) diff --git a/src/corelib/ipc/qsystemsemaphore.cpp b/src/corelib/ipc/qsystemsemaphore.cpp index 0f59b378a30..d22e653b5dd 100644 --- a/src/corelib/ipc/qsystemsemaphore.cpp +++ b/src/corelib/ipc/qsystemsemaphore.cpp @@ -406,12 +406,12 @@ bool QSystemSemaphore::isKeyTypeSupported(QNativeIpcKey::Type type) QNativeIpcKey QSystemSemaphore::platformSafeKey(const QString &key, QNativeIpcKey::Type type) { - return { QtIpcCommon::platformSafeKey(key, IpcType::SystemSemaphore, type), type }; + return QtIpcCommon::platformSafeKey(key, IpcType::SystemSemaphore, type); } QNativeIpcKey QSystemSemaphore::legacyNativeKey(const QString &key, QNativeIpcKey::Type type) { - return { legacyPlatformSafeKey(key, IpcType::SystemSemaphore, type), type }; + return QtIpcCommon::legacyPlatformSafeKey(key, IpcType::SystemSemaphore, type); } QT_END_NAMESPACE diff --git a/src/corelib/ipc/qtipccommon.cpp b/src/corelib/ipc/qtipccommon.cpp index 6629b69a831..f5b576962f3 100644 --- a/src/corelib/ipc/qtipccommon.cpp +++ b/src/corelib/ipc/qtipccommon.cpp @@ -96,11 +96,12 @@ static QNativeIpcKey::Type stringToType(QStringView typeString) On Unix this will be a file name */ -QString QtIpcCommon::legacyPlatformSafeKey(const QString &key, QtIpcCommon::IpcType ipcType, - QNativeIpcKey::Type type) +QNativeIpcKey QtIpcCommon::legacyPlatformSafeKey(const QString &key, QtIpcCommon::IpcType ipcType, + QNativeIpcKey::Type type) { + QNativeIpcKey k(type); if (key.isEmpty()) - return QString(); + return k; QByteArray hex = QCryptographicHash::hash(key.toUtf8(), QCryptographicHash::Sha1).toHex(); @@ -111,13 +112,15 @@ QString QtIpcCommon::legacyPlatformSafeKey(const QString &key, QtIpcCommon::IpcT // to be in the form /. // Since we don't know which application group identifier the user wants // to apply, we instead document that requirement, and use the key directly. - return key; + k.setNativeKey(key); + } else { + // The shared memory name limit on Apple platforms is very low (30 characters), + // so we can't use the logic below of combining the prefix, key, and a hash, + // to ensure a unique and valid name. Instead we use the first part of the + // hash, which should still long enough to avoid collisions in practice. + k.setNativeKey(u'/' + QLatin1StringView(hex).left(SHM_NAME_MAX - 1)); } - // The shared memory name limit on Apple platforms is very low (30 characters), - // so we can't use the logic below of combining the prefix, key, and a hash, - // to ensure a unique and valid name. Instead we use the first part of the - // hash, which should still long enough to avoid collisions in practice. - return u'/' + QLatin1StringView(hex).left(SHM_NAME_MAX - 1); + return k; #endif } @@ -141,38 +144,41 @@ QString QtIpcCommon::legacyPlatformSafeKey(const QString &key, QtIpcCommon::IpcT switch (type) { case QNativeIpcKey::Type::Windows: - if (!isIpcSupported(ipcType, QNativeIpcKey::Type::Windows)) - return QString(); - return result; + if (isIpcSupported(ipcType, QNativeIpcKey::Type::Windows)) + k.setNativeKey(result); + return k; case QNativeIpcKey::Type::PosixRealtime: - if (!isIpcSupported(ipcType, QNativeIpcKey::Type::PosixRealtime)) - return QString(); - return result.prepend(u'/'); + if (isIpcSupported(ipcType, QNativeIpcKey::Type::PosixRealtime)) + k.setNativeKey(result.prepend(u'/')); + return k; case QNativeIpcKey::Type::SystemV: break; } - if (!isIpcSupported(ipcType, QNativeIpcKey::Type::SystemV)) - return QString(); - return QStandardPaths::writableLocation(QStandardPaths::TempLocation) + u'/' + result; + if (isIpcSupported(ipcType, QNativeIpcKey::Type::SystemV)) + k.setNativeKey(QStandardPaths::writableLocation(QStandardPaths::TempLocation) + u'/' + result); + return k; } -QString QtIpcCommon::platformSafeKey(const QString &key, QtIpcCommon::IpcType ipcType, - QNativeIpcKey::Type type) +QNativeIpcKey QtIpcCommon::platformSafeKey(const QString &key, QtIpcCommon::IpcType ipcType, + QNativeIpcKey::Type type) { + QNativeIpcKey k(type); if (key.isEmpty()) - return key; + return k; switch (type) { case QNativeIpcKey::Type::PosixRealtime: - if (!isIpcSupported(ipcType, QNativeIpcKey::Type::PosixRealtime)) - return QString(); + if (isIpcSupported(ipcType, QNativeIpcKey::Type::PosixRealtime)) { #ifdef SHM_NAME_MAX - // The shared memory name limit on Apple platforms is very low (30 - // characters), so we have to cut it down to avoid ENAMETOOLONG. We - // hope that there won't be too many collisions... - return u'/' + QStringView(key).left(SHM_NAME_MAX - 1); + // The shared memory name limit on Apple platforms is very low (30 + // characters), so we have to cut it down to avoid ENAMETOOLONG. We + // hope that there won't be too many collisions... + k.setNativeKey(u'/' + QStringView(key).left(SHM_NAME_MAX - 1)); +#else + k.setNativeKey(u'/' + key); #endif - return u'/' + key; + } + return k; case QNativeIpcKey::Type::Windows: if (isIpcSupported(ipcType, QNativeIpcKey::Type::Windows)) { @@ -197,22 +203,24 @@ QString QtIpcCommon::platformSafeKey(const QString &key, QtIpcCommon::IpcType ip #ifdef MAX_PATH result.truncate(MAX_PATH); #endif - return result; + k.setNativeKey(result); } - return QString(); + return k; case QNativeIpcKey::Type::SystemV: break; } // System V - if (!isIpcSupported(ipcType, QNativeIpcKey::Type::SystemV)) - return QString(); - if (key.startsWith(u'/')) - return key; - - QString baseDir = QStandardPaths::writableLocation(QStandardPaths::RuntimeLocation); - return baseDir + u'/' + key; + if (isIpcSupported(ipcType, QNativeIpcKey::Type::SystemV)) { + if (key.startsWith(u'/')) { + k.setNativeKey(key); + } else { + QString baseDir = QStandardPaths::writableLocation(QStandardPaths::RuntimeLocation); + k.setNativeKey(baseDir + u'/' + key); + } + } + return k; } /*! diff --git a/src/corelib/ipc/qtipccommon_p.h b/src/corelib/ipc/qtipccommon_p.h index 13c76671f64..b2cb3d02e21 100644 --- a/src/corelib/ipc/qtipccommon_p.h +++ b/src/corelib/ipc/qtipccommon_p.h @@ -136,10 +136,8 @@ public: } }; -Q_AUTOTEST_EXPORT QString -legacyPlatformSafeKey(const QString &key, IpcType ipcType, - QNativeIpcKey::Type type = QNativeIpcKey::legacyDefaultTypeForOs()); -Q_AUTOTEST_EXPORT QString platformSafeKey(const QString &key, IpcType ipcType, QNativeIpcKey::Type type); +QNativeIpcKey legacyPlatformSafeKey(const QString &key, IpcType ipcType, QNativeIpcKey::Type type); +QNativeIpcKey platformSafeKey(const QString &key, IpcType ipcType, QNativeIpcKey::Type type); #ifdef Q_OS_UNIX // Convenience function to create the file if needed