From 06e45cbd6a2e09bc32cb1c9aa3779d6bd5282c45 Mon Sep 17 00:00:00 2001 From: Fabian Kosmale Date: Mon, 14 Feb 2022 10:29:24 +0100 Subject: [PATCH] QFileSystemEngine::canonicalName (Unix): clean up control-flow When passing a nullptr to realpath, it will allocate memory. That memory has to be freed (with free) later to avoid a leak, which we so far didn't. This patch ensures that we always clean up the memory by using a unique_ptr. As a drive-by, clean up the control-flow: - Always pass either the stack buffer or nullptr to realpath. - Rely on realpath returning nullptr in the error case. Lastly, fix a few coding-style issues. Change-Id: Ia906df77324020c267b087ec52a9a6c47aaa2a59 Reviewed-by: Thiago Macieira --- src/corelib/io/qfilesystemengine_unix.cpp | 43 ++++++++++++----------- 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/src/corelib/io/qfilesystemengine_unix.cpp b/src/corelib/io/qfilesystemengine_unix.cpp index fb66d47d38..47dd7bea7e 100644 --- a/src/corelib/io/qfilesystemengine_unix.cpp +++ b/src/corelib/io/qfilesystemengine_unix.cpp @@ -58,6 +58,8 @@ #include #include +#include // for std::unique_ptr + #if __has_include() # include #endif @@ -687,39 +689,40 @@ QFileSystemEntry QFileSystemEngine::canonicalName(const QFileSystemEntry &entry, return QFileSystemEntry(slowCanonicalized(absoluteName(entry).filePath())); #else # if defined(Q_OS_DARWIN) || defined(Q_OS_ANDROID) || _POSIX_VERSION < 200801L - char stack_result[PATH_MAX+1]; + // used to store the result of realpath in case where realpath cannot allocate itself + char stack_result[PATH_MAX + 1]; +#else + // enables unconditionally passing stack_result below + std::nullptr_t stack_result = nullptr; +# endif + auto resolved_path_deleter = [&](char *ptr) { + // frees resolved_name if it was allocated by realpath +# if defined(Q_OS_DARWIN) || defined(Q_OS_ANDROID) || _POSIX_VERSION < 200801L + // ptr is either null, or points to stack_result + Q_ASSERT(!ptr || ptr == stack_result); + return; +#else + free(ptr); # endif - char *resolved_name = nullptr; + }; + std::unique_ptr resolved_name {nullptr, resolved_path_deleter}; # if defined(Q_OS_DARWIN) || defined(Q_OS_ANDROID) // On some Android and macOS versions, realpath() will return a path even if // it does not exist. To work around this, we check existence in advance. if (!data.hasFlags(QFileSystemMetaData::ExistsAttribute)) fillMetaData(entry, data, QFileSystemMetaData::ExistsAttribute); - if (!data.exists()) { + if (!data.exists()) errno = ENOENT; - } else { - resolved_name = stack_result; - } - if (resolved_name && realpath(entry.nativeFilePath().constData(), resolved_name) == nullptr) - resolved_name = nullptr; + else + resolved_name.reset(realpath(entry.nativeFilePath().constData(), stack_result)); # else -# if _POSIX_VERSION >= 200801L // ask realpath to allocate memory - resolved_name = realpath(entry.nativeFilePath().constData(), nullptr); -# else - resolved_name = stack_result; - if (realpath(entry.nativeFilePath().constData(), resolved_name) == nullptr) - resolved_name = nullptr; -# endif + resolved_name.reset(realpath(entry.nativeFilePath().constData(), stack_result)); # endif if (resolved_name) { data.knownFlagsMask |= QFileSystemMetaData::ExistsAttribute; data.entryFlags |= QFileSystemMetaData::ExistsAttribute; - QString canonicalPath = QDir::cleanPath(QFile::decodeName(resolved_name)); -# if defined(Q_OS_DARWIN) || defined(Q_OS_ANDROID) || _POSIX_VERSION < 200801L - if (resolved_name != stack_result) - free(resolved_name); -# endif + QString canonicalPath = QDir::cleanPath(QFile::decodeName(resolved_name.get())); return QFileSystemEntry(canonicalPath); } else if (errno == ENOENT || errno == ENOTDIR) { // file doesn't exist data.knownFlagsMask |= QFileSystemMetaData::ExistsAttribute; -- 2.34.1