sal/osl/w32/dllentry.cxx | 5 - sal/osl/w32/file_url.cxx | 119 +++++++++++++++++++++++++++++------------------ sal/osl/w32/file_url.hxx | 4 - sal/osl/w32/process.cxx | 6 -- 4 files changed, 76 insertions(+), 58 deletions(-)
New commits: commit 6e0fa7d4c7b45c98418c289d1d4715eb9eb133f7 Author: Mike Kaganski <mike.kagan...@collabora.com> AuthorDate: Sat Jan 30 00:01:25 2021 +0300 Commit: Mike Kaganski <mike.kagan...@collabora.com> CommitDate: Sat Jan 30 07:08:33 2021 +0100 Don't change process current directory in osl_getAbsoluteFileURL This removes the need to synchronize using global mutex, and allows to get correct result when base directory is unavailable. Change-Id: I9ae70a7d0e8f0840a533a2d0d222336cc0bf0b4e Reviewed-on: https://gerrit.libreoffice.org/c/core/+/110163 Tested-by: Jenkins Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com> diff --git a/sal/osl/w32/dllentry.cxx b/sal/osl/w32/dllentry.cxx index 9493e59bc982..06857df6bbc5 100644 --- a/sal/osl/w32/dllentry.cxx +++ b/sal/osl/w32/dllentry.cxx @@ -88,9 +88,6 @@ static BOOL WINAPI RawDllMain( HINSTANCE, DWORD fdwReason, LPVOID ) /* initialize global mutex */ g_Mutex = osl_createMutex(); - /* initialize "current directory" mutex */ - g_CurrentDirectoryMutex = osl_createMutex(); - g_dwTLSTextEncodingIndex = TlsAlloc(); InitializeCriticalSection( &g_ThreadKeyListCS ); @@ -108,8 +105,6 @@ static BOOL WINAPI RawDllMain( HINSTANCE, DWORD fdwReason, LPVOID ) osl_destroyMutex( g_Mutex ); - osl_destroyMutex( g_CurrentDirectoryMutex ); - /* On a product build memory management finalization might diff --git a/sal/osl/w32/file_url.cxx b/sal/osl/w32/file_url.cxx index a0cb52257b96..e71da15a66b9 100644 --- a/sal/osl/w32/file_url.cxx +++ b/sal/osl/w32/file_url.cxx @@ -28,6 +28,7 @@ #include "file_error.hxx" #include <rtl/alloc.h> +#include <rtl/character.hxx> #include <rtl/ustring.hxx> #include <osl/mutex.h> #include <o3tl/char16_t2wchar_t.hxx> @@ -40,8 +41,6 @@ // FileURL functions -oslMutex g_CurrentDirectoryMutex = nullptr; /* Initialized in dllentry.c */ - static bool IsValidFilePathComponent( sal_Unicode const * lpComponent, sal_Unicode const **lppComponentEnd, DWORD dwFlags) @@ -991,9 +990,40 @@ oslFileError SAL_CALL osl_searchFileURL( return error; } +namespace +{ +bool startsWithDriveColon(const rtl_uString* path) +{ + return rtl::isAsciiAlpha(path->buffer[0]) && path->buffer[1] == ':'; +} +bool startsWithDriveColonSlash(const rtl_uString* path) +{ + return startsWithDriveColon(path) && path->buffer[2] == '\\'; +} +// An absolute path starts either with \\ (an UNC or device path like \\.\ or \\?\) +// or with a ASCII alpha character followed by colon followed by backslash. +bool isAbsolute(const rtl_uString* path) +{ + return (path->buffer[0] == '\\' && path->buffer[1] == '\\') || startsWithDriveColonSlash(path); +} +bool onSameDrive(const rtl_uString* path1, const rtl_uString* path2) +{ + return rtl::toAsciiUpperCase(path1->buffer[0]) == rtl::toAsciiUpperCase(path2->buffer[0]) + && rtl::toAsciiUpperCase(path1->buffer[1]) == rtl::toAsciiUpperCase(path2->buffer[1]); +} +OUString combinePath(rtl_uString* basePath, const sal_Unicode* relPath) +{ + const bool needSep = basePath->buffer[basePath->length - 1] != '\\' && relPath[0] != '\\'; + const auto sSeparator = needSep ? std::u16string_view(u"\\") : std::u16string_view(); + if (basePath->buffer[basePath->length - 1] == '\\' && relPath[0] == '\\') + ++relPath; // avoid two agjacent backslashes + return OUString::unacquired(&basePath) + sSeparator + relPath; +} +} + oslFileError SAL_CALL osl_getAbsoluteFileURL( rtl_uString* ustrBaseURL, rtl_uString* ustrRelativeURL, rtl_uString** pustrAbsoluteURL ) { - oslFileError eError; + oslFileError eError = osl_File_E_None; rtl_uString *ustrRelSysPath = nullptr; rtl_uString *ustrBaseSysPath = nullptr; @@ -1001,64 +1031,63 @@ oslFileError SAL_CALL osl_getAbsoluteFileURL( rtl_uString* ustrBaseURL, rtl_uStr { eError = osl_getSystemPathFromFileURL_( ustrBaseURL, &ustrBaseSysPath, false ); OSL_ENSURE( osl_File_E_None == eError, "osl_getAbsoluteFileURL called with relative or invalid base URL" ); - - eError = osl_getSystemPathFromFileURL_( ustrRelativeURL, &ustrRelSysPath, true ); } - else + if (eError == osl_File_E_None) { - eError = osl_getSystemPathFromFileURL_( ustrRelativeURL, &ustrRelSysPath, false ); + eError = osl_getSystemPathFromFileURL_(ustrRelativeURL, &ustrRelSysPath, + ustrBaseSysPath != nullptr); OSL_ENSURE( osl_File_E_None == eError, "osl_getAbsoluteFileURL called with empty base URL and/or invalid relative URL" ); } if ( !eError ) { - ::osl::LongPathBuffer< sal_Unicode > aBuffer( MAX_LONG_PATH ); - ::osl::LongPathBuffer< sal_Unicode > aCurrentDir( MAX_LONG_PATH ); - LPWSTR lpFilePart = nullptr; - DWORD dwResult; - -/*@@@ToDo - Bad, bad hack, this only works if the base path - really exists which is not necessary according - to RFC2396 - The whole FileURL implementation should be merged - with the rtl/uri class. -*/ - if ( ustrBaseSysPath ) - { - osl_acquireMutex( g_CurrentDirectoryMutex ); - - GetCurrentDirectoryW( aCurrentDir.getBufSizeInSymbols(), o3tl::toW(aCurrentDir) ); - SetCurrentDirectoryW( o3tl::toW(ustrBaseSysPath->buffer) ); - } - - dwResult = GetFullPathNameW( o3tl::toW(ustrRelSysPath->buffer), aBuffer.getBufSizeInSymbols(), o3tl::toW(aBuffer), &lpFilePart ); + OUString sResultPath; - if ( ustrBaseSysPath ) + // If ustrRelSysPath is absolute, we don't need ustrBaseSysPath. + if (ustrBaseSysPath && !isAbsolute(ustrRelSysPath)) { - SetCurrentDirectoryW( o3tl::toW(aCurrentDir) ); + // ustrBaseSysPath is known here to be a valid absolute path -> its first two characters + // are ASCII (either alpha + colon, or double backslashes) - osl_releaseMutex( g_CurrentDirectoryMutex ); - } + // Don't use SetCurrentDirectoryW together with GetFullPathNameW, because: + // (a) it needs synchronization and may affect threads that may access relative paths; + // (b) it would fail or give wrong results for non-existing base path (possible in URL). - if ( dwResult ) - { - if ( dwResult >= aBuffer.getBufSizeInSymbols() ) - eError = osl_File_E_INVAL; - else + if (startsWithDriveColon(ustrRelSysPath)) { - rtl_uString *ustrAbsSysPath = nullptr; - - rtl_uString_newFromStr( &ustrAbsSysPath, aBuffer ); - - eError = osl_getFileURLFromSystemPath( ustrAbsSysPath, pustrAbsoluteURL ); + // Special case: a path relative to a specific drive's current directory. + // Should we error out here? - if ( ustrAbsSysPath ) - rtl_uString_release( ustrAbsSysPath ); + if (onSameDrive(ustrRelSysPath, ustrBaseSysPath)) + { + // If ustrBaseSysPath defines a path on the same drive, combine them + sResultPath = combinePath(ustrBaseSysPath, ustrRelSysPath->buffer + 2); + } + else + { + // Otherwise just call GetFullPathNameW to get current directory on that drive + osl::LongPathBuffer<wchar_t> aBuf(MAX_LONG_PATH); + DWORD dwResult = GetFullPathNameW(o3tl::toW(ustrRelSysPath->buffer), + aBuf.getBufSizeInSymbols(), aBuf, nullptr); + if (dwResult) + { + if (dwResult >= aBuf.getBufSizeInSymbols()) + eError = osl_File_E_INVAL; + else + sResultPath = o3tl::toU(aBuf); + } + else + eError = oslTranslateFileError(GetLastError()); + } } + else + sResultPath = combinePath(ustrBaseSysPath, ustrRelSysPath->buffer); } else - eError = oslTranslateFileError( GetLastError() ); + sResultPath = OUString::unacquired(&ustrRelSysPath); + + if (eError == osl_File_E_None) + eError = osl_getFileURLFromSystemPath(sResultPath.pData, pustrAbsoluteURL); } if ( ustrBaseSysPath ) diff --git a/sal/osl/w32/file_url.hxx b/sal/osl/w32/file_url.hxx index 37bf86ca3b12..22ad1b89af81 100644 --- a/sal/osl/w32/file_url.hxx +++ b/sal/osl/w32/file_url.hxx @@ -23,7 +23,7 @@ #include <sal/types.h> #include <rtl/ustring.h> #include <osl/file.h> -#include <osl/mutex.h> +#include <osl/mutex.hxx> #if !defined WIN32_LEAN_AND_MEAN # define WIN32_LEAN_AND_MEAN @@ -65,8 +65,6 @@ oslFileError osl_getSystemPathFromFileURL_ ( bool bAllowRelative ); -extern oslMutex g_CurrentDirectoryMutex; - #endif /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sal/osl/w32/process.cxx b/sal/osl/w32/process.cxx index 205d6415e035..065415f2cf86 100644 --- a/sal/osl/w32/process.cxx +++ b/sal/osl/w32/process.cxx @@ -471,11 +471,7 @@ oslProcessError SAL_CALL osl_clearEnvironment(rtl_uString *ustrVar) oslProcessError SAL_CALL osl_getProcessWorkingDir( rtl_uString **pustrWorkingDir ) { ::osl::LongPathBuffer< sal_Unicode > aBuffer( MAX_LONG_PATH ); - DWORD dwLen = 0; - - osl_acquireMutex( g_CurrentDirectoryMutex ); - dwLen = GetCurrentDirectoryW( aBuffer.getBufSizeInSymbols(), o3tl::toW(aBuffer) ); - osl_releaseMutex( g_CurrentDirectoryMutex ); + DWORD dwLen = GetCurrentDirectoryW(aBuffer.getBufSizeInSymbols(), o3tl::toW(aBuffer)); if ( dwLen && dwLen < aBuffer.getBufSizeInSymbols() ) { _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits