[Libreoffice-commits] core.git: comphelper/source dbaccess/win32 desktop/win32 extensions/source include/comphelper include/LibreOfficeKit sal/osl shell/source vcl/win

Mike Kaganski mike.kaganski at collabora.com
Sat Nov 4 14:25:04 UTC 2017


 comphelper/source/windows/windows_process.cxx   |    2 +-
 dbaccess/win32/source/odbcconfig/odbcconfig.cxx |    7 +++----
 desktop/win32/source/applauncher/launcher.cxx   |    7 +++----
 desktop/win32/source/loader.cxx                 |    4 ++--
 extensions/source/activex/SOActiveX.cxx         |    7 +++----
 include/LibreOfficeKit/LibreOfficeKitInit.h     |    3 ++-
 include/comphelper/windowserrorstring.hxx       |    4 ++--
 sal/osl/w32/socket.cxx                          |    9 ++-------
 shell/source/win32/spsupp/COMOpenDocuments.cxx  |    4 ++--
 vcl/win/window/salobj.cxx                       |   14 ++++----------
 10 files changed, 24 insertions(+), 37 deletions(-)

New commits:
commit e3530d2c9d5dc98c6bacf243c163d651624e1ba6
Author: Mike Kaganski <mike.kaganski at collabora.com>
Date:   Thu Nov 2 13:22:41 2017 +0300

    Make Windows error reporting more robust
    
    https://msdn.microsoft.com/en-us/library/ms679351 describes that
    "it is unsafe to take an arbitrary system error code returned from an API
    and use FORMAT_MESSAGE_FROM_SYSTEM without FORMAT_MESSAGE_IGNORE_INSERTS"
    Previously in case when an error string would contain inserts, function
    returned error, so the error message wasn't shown (at least it didn't
    crash, thanks to nullptr as the function's last argument).
    
    As the function may fail, we now pre-nullify the buffer pointer to avoid
    dereferencing uninitialized pointer later (though at least for some
    Windows versions, the function nullifies the pointer in case of
    FORMAT_MESSAGE_ALLOCATE_BUFFER, but there's no explicit guarantee of this).
    
    Also release of allocated buffer is changed to recommended use of
    HeapFree.
    
    The code that doesn't make use of OUString is left directly calling
    FormatMessage, to avoid introducing new dependencies. Where it makes
    sense, we now use WindowsErrorString from <comphelper/windowserrorstring.hxx>
    
    Change-Id: I834c08eb6d92987e7d3d01e2c36ec55e42aea848
    Reviewed-on: https://gerrit.libreoffice.org/44206
    Tested-by: Jenkins <ci at libreoffice.org>
    Reviewed-by: Noel Grandin <noel.grandin at collabora.co.uk>
    Reviewed-by: Mike Kaganski <mike.kaganski at collabora.com>

diff --git a/comphelper/source/windows/windows_process.cxx b/comphelper/source/windows/windows_process.cxx
index c58a0d52d7f4..528fb6b1aaf0 100644
--- a/comphelper/source/windows/windows_process.cxx
+++ b/comphelper/source/windows/windows_process.cxx
@@ -254,7 +254,7 @@ WinLaunchChild(const wchar_t *exePath,
                        nullptr);
         wprintf(L"Error restarting: %s\n", lpMsgBuf ? lpMsgBuf : L"(null)");
         if (lpMsgBuf)
-            LocalFree(lpMsgBuf);
+            HeapFree(GetProcessHeap(), 0, lpMsgBuf);
     }
 
     free(cl);
diff --git a/dbaccess/win32/source/odbcconfig/odbcconfig.cxx b/dbaccess/win32/source/odbcconfig/odbcconfig.cxx
index 86f8e67c4071..73eb04a74d57 100644
--- a/dbaccess/win32/source/odbcconfig/odbcconfig.cxx
+++ b/dbaccess/win32/source/odbcconfig/odbcconfig.cxx
@@ -44,10 +44,9 @@ int displayLastError()
 {
     DWORD   dwError = GetLastError();
 
-    LPVOID lpMsgBuf;
+    LPVOID lpMsgBuf = nullptr;
     FormatMessageW(
-        FORMAT_MESSAGE_ALLOCATE_BUFFER |
-        FORMAT_MESSAGE_FROM_SYSTEM,
+        FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
         nullptr,
         dwError,
         MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), // Default language
@@ -60,7 +59,7 @@ int displayLastError()
     MessageBoxW( nullptr, static_cast<LPCWSTR>(lpMsgBuf), nullptr, MB_OK | MB_ICONERROR );
 
     // Free the buffer.
-    LocalFree( lpMsgBuf );
+    HeapFree( GetProcessHeap(), 0, lpMsgBuf );
 
     return dwError;
 }
diff --git a/desktop/win32/source/applauncher/launcher.cxx b/desktop/win32/source/applauncher/launcher.cxx
index d3114aa2d804..987b3e2985e1 100644
--- a/desktop/win32/source/applauncher/launcher.cxx
+++ b/desktop/win32/source/applauncher/launcher.cxx
@@ -81,11 +81,10 @@ extern "C" int APIENTRY wWinMain( HINSTANCE, HINSTANCE, LPWSTR, int )
 
     DWORD dwError = GetLastError();
 
-    LPWSTR lpMsgBuf;
+    LPWSTR lpMsgBuf = nullptr;
 
     FormatMessageW(
-        FORMAT_MESSAGE_ALLOCATE_BUFFER |
-        FORMAT_MESSAGE_FROM_SYSTEM,
+        FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
         nullptr,
         dwError,
         MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), // Default language
@@ -98,7 +97,7 @@ extern "C" int APIENTRY wWinMain( HINSTANCE, HINSTANCE, LPWSTR, int )
     MessageBoxW( nullptr, lpMsgBuf, nullptr, MB_OK | MB_ICONERROR );
 
     // Free the buffer.
-    LocalFree( lpMsgBuf );
+    HeapFree( GetProcessHeap(), 0, lpMsgBuf );
 
     return dwError;
 }
diff --git a/desktop/win32/source/loader.cxx b/desktop/win32/source/loader.cxx
index 72bcafc50457..4425c1e697d9 100644
--- a/desktop/win32/source/loader.cxx
+++ b/desktop/win32/source/loader.cxx
@@ -27,10 +27,10 @@ void fail()
 {
     LPWSTR buf = nullptr;
     FormatMessageW(
-        FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM, nullptr,
+        FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, nullptr,
         GetLastError(), 0, reinterpret_cast< LPWSTR >(&buf), 0, nullptr);
     MessageBoxW(nullptr, buf, nullptr, MB_OK | MB_ICONERROR);
-    LocalFree(buf);
+    HeapFree(GetProcessHeap(), 0, buf);
     TerminateProcess(GetCurrentProcess(), 255);
 }
 
diff --git a/extensions/source/activex/SOActiveX.cxx b/extensions/source/activex/SOActiveX.cxx
index 1105dac8561a..c2e1fbb74ca4 100644
--- a/extensions/source/activex/SOActiveX.cxx
+++ b/extensions/source/activex/SOActiveX.cxx
@@ -44,10 +44,9 @@
 
 void OutputError_Impl( HWND hw, HRESULT ErrorCode )
 {
-    LPWSTR sMessage;
+    LPWSTR sMessage = nullptr;
     FormatMessageW(
-        FORMAT_MESSAGE_ALLOCATE_BUFFER |
-        FORMAT_MESSAGE_FROM_SYSTEM,
+        FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
         nullptr,
         ErrorCode,
         MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), // Default language
@@ -56,7 +55,7 @@ void OutputError_Impl( HWND hw, HRESULT ErrorCode )
         nullptr
     );
     MessageBoxW( hw, sMessage, nullptr, MB_OK | MB_ICONINFORMATION );
-    LocalFree( sMessage );
+    HeapFree( GetProcessHeap(), 0, sMessage );
 }
 
 HRESULT ExecuteFunc( IDispatch* idispUnoObject,
diff --git a/include/LibreOfficeKit/LibreOfficeKitInit.h b/include/LibreOfficeKit/LibreOfficeKitInit.h
index c82fb7b08d1d..f95ee49634b5 100644
--- a/include/LibreOfficeKit/LibreOfficeKitInit.h
+++ b/include/LibreOfficeKit/LibreOfficeKitInit.h
@@ -100,7 +100,8 @@ extern "C"
     static char *lok_dlerror(void)
     {
         LPSTR buf = NULL;
-        FormatMessageA(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM, NULL, GetLastError(), 0, reinterpret_cast<LPSTR>(&buf), 0, NULL);
+        FormatMessageA(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
+            NULL, GetLastError(), 0, reinterpret_cast<LPSTR>(&buf), 0, NULL);
         return buf;
     }
 
diff --git a/include/comphelper/windowserrorstring.hxx b/include/comphelper/windowserrorstring.hxx
index bb4b16241ca5..1b50f1eadb58 100644
--- a/include/comphelper/windowserrorstring.hxx
+++ b/include/comphelper/windowserrorstring.hxx
@@ -21,7 +21,7 @@ inline OUString WindowsErrorString(DWORD nErrorCode)
 {
     LPWSTR pMsgBuf;
 
-    if (FormatMessageW(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM,
+    if (FormatMessageW(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
                        nullptr,
                        nErrorCode,
                        MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
@@ -33,7 +33,7 @@ inline OUString WindowsErrorString(DWORD nErrorCode)
     OUString result(o3tl::toU(pMsgBuf));
     result.endsWith("\r\n", &result);
 
-    LocalFree(pMsgBuf);
+    HeapFree(GetProcessHeap(), 0, pMsgBuf);
 
     return result;
 }
diff --git a/sal/osl/w32/socket.cxx b/sal/osl/w32/socket.cxx
index 9c6715be2627..b3d1e9f52e63 100644
--- a/sal/osl/w32/socket.cxx
+++ b/sal/osl/w32/socket.cxx
@@ -24,6 +24,7 @@
 #include <rtl/alloc.h>
 #include <sal/log.hxx>
 #include <o3tl/char16_t2wchar_t.hxx>
+#include <comphelper/windowserrorstring.hxx>
 
 #include "sockimpl.hxx"
 
@@ -791,13 +792,7 @@ oslSocket SAL_CALL osl_createSocket(
     if(pSocket->m_Socket == OSL_INVALID_SOCKET)
     {
         int nErrno = WSAGetLastError();
-        wchar_t *sErr = nullptr;
-        FormatMessageW(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
-                       nullptr, nErrno,
-                       MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
-                       reinterpret_cast<LPWSTR>(&sErr), 0, nullptr);
-        SAL_WARN("sal.osl", "socket creation failed: (" << nErrno << ") " << sErr);
-        LocalFree(sErr);
+        SAL_WARN("sal.osl", "socket creation failed: (" << nErrno << "): " << WindowsErrorString(nErrno));
 
         destroySocketImpl(pSocket);
         pSocket = nullptr;
diff --git a/shell/source/win32/spsupp/COMOpenDocuments.cxx b/shell/source/win32/spsupp/COMOpenDocuments.cxx
index 3412598d94c2..829d1923fe5f 100644
--- a/shell/source/win32/spsupp/COMOpenDocuments.cxx
+++ b/shell/source/win32/spsupp/COMOpenDocuments.cxx
@@ -51,7 +51,7 @@ HRESULT LOStart(const wchar_t* sModeArg, const wchar_t* sFilePath, bool bDoSecur
     if (CreateProcessW(nullptr, sCommandLine, nullptr, nullptr, FALSE, 0, nullptr, nullptr, &si, &pi) == FALSE)
     {
         DWORD dwError = GetLastError();
-        wchar_t* sMsgBuf;
+        wchar_t* sMsgBuf = nullptr;
         FormatMessageW(
             FORMAT_MESSAGE_ALLOCATE_BUFFER |
             FORMAT_MESSAGE_FROM_SYSTEM |
@@ -65,7 +65,7 @@ HRESULT LOStart(const wchar_t* sModeArg, const wchar_t* sFilePath, bool bDoSecur
         size_t nBufSize = wcslen(sMsgBuf) + 100;
         std::vector<wchar_t> sDisplayBuf(nBufSize);
         swprintf(sDisplayBuf.data(), nBufSize, L"Could not start LibreOffice. Error is 0x%08X:\n\n%s", dwError, sMsgBuf);
-        LocalFree(sMsgBuf);
+        HeapFree(GetProcessHeap(), 0, sMsgBuf);
 
         // Report the error to user and return error
         MessageBoxW(nullptr, sDisplayBuf.data(), nullptr, MB_ICONERROR);
diff --git a/vcl/win/window/salobj.cxx b/vcl/win/window/salobj.cxx
index 75e406c33f61..0996e9c5eb13 100644
--- a/vcl/win/window/salobj.cxx
+++ b/vcl/win/window/salobj.cxx
@@ -29,6 +29,8 @@
 #include <win/salframe.h>
 #include <win/salobj.h>
 
+#include <comphelper/windowserrorstring.hxx>
+
 static bool ImplIsSysWindowOrChild( HWND hWndParent, HWND hWndChild )
 {
     if ( hWndParent == hWndChild )
@@ -522,16 +524,8 @@ SalObject* ImplSalCreateObject( WinSalInstance* pInst, WinSalFrame* pParent )
 
         if ( !hWndChild )
         {
-#if OSL_DEBUG_LEVEL > 1
-            wchar_t *msg = NULL;
-            FormatMessageW(FORMAT_MESSAGE_ALLOCATE_BUFFER
-                          |FORMAT_MESSAGE_IGNORE_INSERTS
-                          |FORMAT_MESSAGE_FROM_SYSTEM,
-                           NULL, GetLastError(), 0,
-                           (LPWSTR) &msg, 0, NULL);
-            MessageBoxW(NULL, msg, L"CreateWindowExW failed", MB_OK);
-            HeapFree(GetProcessHeap(), msg);
-#endif
+            SAL_WARN("vcl", "CreateWindowExW failed: " << WindowsErrorString(GetLastError()));
+
             delete pObject;
             return nullptr;
         }


More information about the Libreoffice-commits mailing list