Use sal::systools::COMReference in getAdoDatalink

Change-Id: If0c474209da5390c0c6e28c11ca26a1c915ab51f
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/127218
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>

2 files changed, 95 insertions, 125 deletions

diff --git a/dbaccess/source/ui/dlg/adodatalinks.cxx b/dbaccess/source/ui/dlg/adodatalinks.cxx
index 74703b010bcb..be60cdb9b089 100644
--- a/dbaccess/source/ui/dlg/adodatalinks.cxx
+++ b/dbaccess/source/ui/dlg/adodatalinks.cxx
@@ -26,6 +26,7 @@
 
 #include <comphelper/scopeguard.hxx>
 #include <o3tl/char16_t2wchar_t.hxx>
+#include <systools/win32/comtools.hxx>
 
 #include <initguid.h>
 #include <adoid.h>
@@ -37,154 +38,90 @@ namespace {
 
 OUString PromptNew(sal_IntPtr hWnd)
 {
-    HRESULT hr;
-    IDataSourceLocator* dlPrompt = nullptr;
-    ADOConnection* piTmpConnection = nullptr;
-    BSTR _result=nullptr;
-
-    // Initialize COM
-    hr = ::CoInitializeEx( nullptr, COINIT_APARTMENTTHREADED );
-    if (FAILED(hr) && hr != RPC_E_CHANGED_MODE)
-        std::abort();
-    const bool bDoUninit = SUCCEEDED(hr);
-    comphelper::ScopeGuard g([bDoUninit] () {
-        if (bDoUninit)
-            CoUninitialize();
-    });
-
-    // Instantiate DataLinks object.
-    hr = CoCreateInstance(
-                    CLSID_DataLinks,                //clsid -- Data Links UI
-                    nullptr,                        //pUnkOuter
-                    CLSCTX_INPROC_SERVER,           //dwClsContext
-                    IID_IDataSourceLocator,     //riid
-                    reinterpret_cast<void**>(&dlPrompt)   //ppvObj
-                    );
-    if( FAILED( hr ) )
+    try
     {
-        return OUString();
-    }
+        // Initialize COM
+        sal::systools::CoInitializeGuard aGuard(COINIT_APARTMENTTHREADED);
 
-    dlPrompt->put_hWnd(hWnd);
-    if( FAILED( hr ) )
-    {
-        dlPrompt->Release( );
-        return OUString();
-    }
+        // Instantiate DataLinks object.
+        sal::systools::COMReference<IDataSourceLocator> dlPrompt;
+        dlPrompt.CoCreateInstance(CLSID_DataLinks,       //clsid -- Data Links UI
+                                  nullptr,               //pUnkOuter
+                                  CLSCTX_INPROC_SERVER); //dwClsContext
 
-    // Prompt for connection information.
-    hr = dlPrompt->PromptNew(reinterpret_cast<IDispatch **>(&piTmpConnection));
+        sal::systools::ThrowIfFailed(dlPrompt->put_hWnd(hWnd), "put_hWnd failed");
 
-    if( FAILED( hr ) || !piTmpConnection )
-    {
-        dlPrompt->Release( );
-        return OUString();
-    }
+        // Prompt for connection information.
+        sal::systools::COMReference<IDispatch> piDispatch;
+        sal::systools::ThrowIfFailed(dlPrompt->PromptNew(&piDispatch), "PromptNew failed");
+        sal::systools::COMReference<ADOConnection> piTmpConnection(piDispatch,
+                                                                   sal::systools::COM_QUERY_THROW);
+
+        BSTR _result = nullptr;
+        sal::systools::ThrowIfFailed(piTmpConnection->get_ConnectionString(&_result),
+                                     "get_ConnectionString failed");
 
-    hr = piTmpConnection->get_ConnectionString(&_result);
-    if( FAILED( hr ) )
+        // FIXME: Don't we need SysFreeString(_result)?
+        return OUString(o3tl::toU(_result), SysStringLen(_result));
+    }
+    catch (const sal::systools::ComError&)
     {
-        piTmpConnection->Release( );
-        dlPrompt->Release( );
         return OUString();
     }
-
-    piTmpConnection->Release( );
-    dlPrompt->Release( );
-    // Don't we need SysFreeString(_result)?
-    return OUString(o3tl::toU(_result));
 }
 
 OUString PromptEdit(sal_IntPtr hWnd, OUString const & connstr)
 {
-    HRESULT hr;
-    IDataSourceLocator* dlPrompt = nullptr;
-    ADOConnection* piTmpConnection = nullptr;
-    BSTR _result=nullptr;
-
-    // Initialize COM
-    ::CoInitializeEx( nullptr, COINIT_APARTMENTTHREADED );
-
-    hr = CoCreateInstance(CLSID_CADOConnection,
-                nullptr,
-                CLSCTX_INPROC_SERVER,
-                IID_IADOConnection,
-                reinterpret_cast<LPVOID *>(&piTmpConnection));
-    if( FAILED( hr ) )
-    {
-        piTmpConnection->Release( );
-        return connstr;
-    }
-
-
-    hr = piTmpConnection->put_ConnectionString(
-        const_cast<BSTR>(o3tl::toW(connstr.getStr())));
-    if( FAILED( hr ) )
+    try
     {
-        piTmpConnection->Release( );
-        return connstr;
-    }
+        // Initialize COM
+        sal::systools::CoInitializeGuard aGuard(COINIT_APARTMENTTHREADED);
 
-    // Instantiate DataLinks object.
-    hr = CoCreateInstance(
-                    CLSID_DataLinks,                //clsid -- Data Links UI
-                    nullptr,                        //pUnkOuter
-                    CLSCTX_INPROC_SERVER,           //dwClsContext
-                    IID_IDataSourceLocator,     //riid
-                    reinterpret_cast<void**>(&dlPrompt) //ppvObj
-                    );
-    if( FAILED( hr ) )
-    {
-        piTmpConnection->Release( );
-        dlPrompt->Release( );
-        return connstr;
-    }
+        sal::systools::COMReference<ADOConnection> piTmpConnection;
+        piTmpConnection.CoCreateInstance(CLSID_CADOConnection, nullptr, CLSCTX_INPROC_SERVER);
 
-    dlPrompt->put_hWnd(hWnd);
-    if( FAILED( hr ) )
-    {
-        piTmpConnection->Release( );
-        dlPrompt->Release( );
-        return connstr;
-    }
+        // FIXME: BSTR is not just cast from a random string
+        sal::systools::ThrowIfFailed(
+            piTmpConnection->put_ConnectionString(const_cast<BSTR>(o3tl::toW(connstr.getStr()))),
+            "put_ConnectionString failed");
 
-    VARIANT_BOOL pbSuccess;
+        // Instantiate DataLinks object.
+        sal::systools::COMReference<IDataSourceLocator> dlPrompt;
+        dlPrompt.CoCreateInstance(CLSID_DataLinks,       //clsid -- Data Links UI
+                                  nullptr,               //pUnkOuter
+                                  CLSCTX_INPROC_SERVER); //dwClsContext
 
-    // Prompt for connection information.
-    hr = dlPrompt->PromptEdit(reinterpret_cast<IDispatch **>(&piTmpConnection),&pbSuccess);
-    if( SUCCEEDED( hr ) && !pbSuccess ) //if user press cancel then sal_False == pbSuccess
-    {
-        piTmpConnection->Release( );
-        dlPrompt->Release( );
-        return connstr;
-    }
+        sal::systools::ThrowIfFailed(dlPrompt->put_hWnd(hWnd), "put_hWnd failed");
 
-    if( FAILED( hr ) )
-    {
-        // Prompt for new connection information.
-        piTmpConnection->Release( );
-        piTmpConnection = nullptr;
-        hr = dlPrompt->PromptNew(reinterpret_cast<IDispatch **>(&piTmpConnection));
-        if(  FAILED( hr ) || !piTmpConnection )
+        try
         {
-            dlPrompt->Release( );
-            return connstr;
+            // Prompt for connection information.
+            IDispatch* piDispatch = piTmpConnection.get();
+            VARIANT_BOOL pbSuccess;
+            sal::systools::ThrowIfFailed(dlPrompt->PromptEdit(&piDispatch, &pbSuccess),
+                                         "PromptEdit failed");
+            if (!pbSuccess) //if user press cancel then sal_False == pbSuccess
+                return connstr;
         }
-    }
+        catch (const sal::systools::ComError&)
+        {
+            // Prompt for new connection information.
+            sal::systools::COMReference<IDispatch> piDispatch;
+            sal::systools::ThrowIfFailed(dlPrompt->PromptNew(&piDispatch), "PromptNew failed");
+            piTmpConnection.set(piDispatch, sal::systools::COM_QUERY_THROW);
+        }
+
+        BSTR _result = nullptr;
+        sal::systools::ThrowIfFailed(piTmpConnection->get_ConnectionString(&_result),
+                                     "get_ConnectionString failed");
 
-    hr = piTmpConnection->get_ConnectionString(&_result);
-    if( FAILED( hr ) )
+        // FIXME: Don't we need SysFreeString(_result)?
+        return OUString(o3tl::toU(_result), SysStringLen(_result));
+    }
+    catch (const sal::systools::ComError&)
     {
-        piTmpConnection->Release( );
-        dlPrompt->Release( );
         return connstr;
     }
-
-    piTmpConnection->Release( );
-    dlPrompt->Release( );
-    CoUninitialize();
-    // Don't we need SysFreeString(_result)?
-    return OUString(o3tl::toU(_result));
 }
 
 }
diff --git a/include/systools/win32/comtools.hxx b/include/systools/win32/comtools.hxx
index 2e19f1f9e677..c6f9c4f200f7 100644
--- a/include/systools/win32/comtools.hxx
+++ b/include/systools/win32/comtools.hxx
@@ -20,6 +20,7 @@
 #pragma once
 
 #include <string>
+#include <string_view>
 #include <stdexcept>
 #include <type_traits>
 #include <utility>
@@ -45,6 +46,38 @@ namespace sal::systools
         HRESULT hr_;
     };
 
+    /* Convert failed HRESULT to thrown ComError */
+    inline void ThrowIfFailed(HRESULT hr, std::string_view msg)
+    {
+        if (FAILED(hr))
+            throw ComError(std::string(msg), hr);
+    }
+
+    /* A guard class to call CoInitializeEx/CoUninitialize in proper pairs
+     * See also: o3tl::safeCoInitializeEx doing dangerous re-initialization
+     */
+    class CoInitializeGuard
+    {
+    public:
+        explicit CoInitializeGuard(DWORD dwCoInit, bool bThrowOnChangeMode = false)
+        {
+            HRESULT hr = ::CoInitializeEx(nullptr, dwCoInit);
+            if (FAILED(hr) && (bThrowOnChangeMode || hr != RPC_E_CHANGED_MODE))
+                throw ComError("CoInitializeEx failed", hr);
+            mbUninit = SUCCEEDED(hr);
+        }
+        CoInitializeGuard(const CoInitializeGuard&) = delete; // non-construction-copyable
+        void operator=(const CoInitializeGuard&) = delete; // non-copyable
+        ~CoInitializeGuard()
+        {
+            if (mbUninit)
+                CoUninitialize();
+        }
+
+    private:
+        bool mbUninit;
+    };
+
     struct COM_QUERY_TAG {} constexpr COM_QUERY;
     struct COM_QUERY_THROW_TAG {} constexpr COM_QUERY_THROW;
     template <typename TAG>
@@ -108,7 +141,7 @@ namespace sal::systools
         COMReference<T2> QueryInterface(TAG) const
         {
             void* ip = nullptr;
-            HRESULT hr = E_FAIL;
+            HRESULT hr = E_POINTER;
             if (com_ptr_)
                 hr = com_ptr_->QueryInterface(__uuidof(T2), &ip);