Some DDE fixes

It seems that some DDE code was virtually never tested in our CI. That resulted in several unnoticed regressions. In commit 81b954718f (loplugin:unnecessaryvirtuals, 2015-06-09), DdeService::MakeTopic was made non-virtual. That broke its use on Windows, where a derived class used to override that method (which of course didn't use the 'override' keyvord, hence went unnoticed). In commit 88e3b846b8 (remove some dead bits of DDE, 2016-04-06), DdeService::MakeTopic was finally dropped. In commit c026b93640 (no need to allocate SfxDdeDocTopics_Impl separately, 2023-02-28), a check in SfxApplication::AddDdeTopic was changed from testing if pDocTopics were initialized to checking if they are empty. The new test made adding anything to the vector impossible. This change tried to make it functional again, and adds a (Windows- only) unit test. To enable unit test, we need an app name, so that DDE is initialized properly. Also, ImplDdeService::MakeTopic had a condition to exit early, when !Application::IsInExecute(). Here we allow to add it, if unit test is running. SfxDdeDocTopic_Impl::Put ismodified to store OUString to the Any - see ScByteSequenceToString::GetString. Without the change, UTF-16 was treated as 8-bit-encoded string. theDdeInstData in svl/source/svdde/ddecli.cxx is now not a pointer, bus a function-local static. No need to delete it: it has nothing that needs to be released. Before, it could get destroyed too early, preventing DDE from working. DdeConnection::~DdeConnection could reliably deadlock, when it had a connection to the server in the same process; processEventsToIdle is called now to avoid that. DdeData ctor taking OUString is fixed: xImp->nData is bytes, not characters. Change-Id: I9a151eebc65a26b76c9f6134e97cd1f4d2e2b615 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/192640 Tested-by: Jenkins Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
2025-10-19 10:43:41 +02:00
parent 802a0bea1e
commit fb347e61c9
9 changed files with 194 additions and 143 deletions
@@ -277,6 +277,10 @@ class SVL_DLLPUBLIC DdeService
 {
    friend class    DdeInternal;

+public:
+    // Eventually creating a new item. return false -> Topic creation failed
+    virtual bool MakeTopic(const OUString& rItem) = 0;
+
 protected:
    OUString Topics();
    OUString Formats();
@@ -13,6 +13,7 @@
 #include <svx/svdpage.hxx>
 #include <comphelper/processfactory.hxx>
 #include <comphelper/propertyvalue.hxx>
+#include <vcl/svapp.hxx>

 #include <conditio.hxx>
 #include <document.hxx>
@@ -37,6 +38,7 @@ class ScMacrosTest : public ScModelTestBase
 {
 public:
    ScMacrosTest();
+    virtual void setUp() override;
 };

 // I suppose you could say this test doesn't really belong here, OTOH
@@ -1082,11 +1084,37 @@ CPPUNIT_TEST_FIXTURE(ScMacrosTest, testTdf168750)
    CPPUNIT_ASSERT_EQUAL(u"FALSE"_ustr, pDoc->GetString(ScAddress(1, 0, 0)));
 }

+#if defined(_WIN32) // DDE calls work only on Windows currently
+CPPUNIT_TEST_FIXTURE(ScMacrosTest, testDdePoke)
+{
+    // Get a document with a text in A1, and use its Basic macro to send that text into the
+    // same document's B2 using DDEPoke call:
+    createScDoc("DdePoke.fods");
+    ScDocument* pDoc = getScDoc();
+    // A1 has a text:
+    CPPUNIT_ASSERT_EQUAL(u"Hello from Sender"_ustr, pDoc->GetString(ScAddress(0, 0, 0)));
+    // B2 is empty initially:
+    CPPUNIT_ASSERT(pDoc->GetString(ScAddress(1, 1, 0)).isEmpty());
+
+    executeMacro(u"vnd.sun.star.script:Standard.Module1.SendDataWithDDEPoke"
+                 "?language=Basic&location=document"_ustr);
+
+    // B2 has the expected text now:
+    CPPUNIT_ASSERT_EQUAL(u"Hello from Sender"_ustr, pDoc->GetString(ScAddress(1, 1, 0)));
+}
+#endif
+
 ScMacrosTest::ScMacrosTest()
      : ScModelTestBase(u"/sc/qa/extras/testdocuments"_ustr)
 {
 }

+void ScMacrosTest::setUp()
+{
+    Application::SetAppName("CppunitTest_sc_macros_test");
+    ScModelTestBase::setUp();
+}
+
 CPPUNIT_PLUGIN_IMPLEMENT();

 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
@@ -0,0 +1,39 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<office:document xmlns:office="urn:oasis:names:tc:opendocument:xmlns:office:1.0" xmlns:ooo="http://openoffice.org/2004/office" xmlns:text="urn:oasis:names:tc:opendocument:xmlns:text:1.0" xmlns:table="urn:oasis:names:tc:opendocument:xmlns:table:1.0" xmlns:script="urn:oasis:names:tc:opendocument:xmlns:script:1.0" office:version="1.4" office:mimetype="application/vnd.oasis.opendocument.spreadsheet">
+ <office:scripts>
+  <office:script script:language="ooo:Basic">
+   <ooo:libraries>
+    <ooo:library-embedded ooo:name="Standard">
+     <ooo:module ooo:name="Module1">
+      <ooo:source-code>Sub SendDataWithDDEPoke()
+    dataToSend = &quot;ID;P&quot; + Chr(10) + &quot;C;Y1;X1;K&quot;&quot;&quot; + ThisComponent.Sheets(0).getCellRangeByName(&quot;A1&quot;).String + &quot;&quot;&quot;&quot;
+
+    DDEChannel = DDEInitiate(&quot;CppunitTest_sc_macros_test&quot;, ConvertFromURL(ThisComponent.URL))
+
+    &apos; Define the type of the data in the following call
+    DDEPoke DDEChannel, &quot;Format&quot;, &quot;SYLK&quot;
+    &apos; Send the data to the specified cell
+    DDEPoke DDEChannel, &quot;Sheet1.B2&quot;, dataToSend
+    
+    &apos;DDETerminate DDEChannel
+End Sub
+      </ooo:source-code>
+     </ooo:module>
+    </ooo:library-embedded>
+   </ooo:libraries>
+  </office:script>
+ </office:scripts>
+ <office:body>
+  <office:spreadsheet>
+   <table:table table:name="Sheet1">
+    <table:table-column/>
+    <table:table-row>
+     <table:table-cell office:value-type="string">
+      <text:p>Hello from Sender</text:p>
+     </table:table-cell>
+    </table:table-row>
+   </table:table>
+  </office:spreadsheet>
+ </office:body>
+</office:document>
@@ -33,6 +33,7 @@
 #include <sfx2/lnkbase.hxx>
 #include <sfx2/linkmgr.hxx>

+#include <o3tl/test_info.hxx>
 #include <tools/debug.hxx>
 #include <tools/urlobj.hxx>
 #include <comphelper/diagnose_ex.hxx>
@@ -73,7 +74,7 @@ public:
    explicit ImplDdeService( const OUString& rNm )
        : DdeService( rNm )
    {}
-    virtual bool MakeTopic( const OUString& );
+    virtual bool MakeTopic( const OUString& ) override;

    virtual OUString  Topics();

@@ -106,7 +107,7 @@ bool ImplDdeService::MakeTopic( const OUString& rNm )
 {
    // Workaround for Event after Main() under OS/2
    // happens when exiting starts the App again
-    if ( !Application::IsInExecute() )
+    if ( !Application::IsInExecute() && !o3tl::IsRunningUnitTest() )
        return false;

    // The Topic rNm is sought, do we have it?
@@ -443,7 +444,7 @@ void SfxAppData_Impl::DeInitDDE()
 void SfxApplication::AddDdeTopic( SfxObjectShell* pSh )
 {
    //OV: DDE is disconnected in server mode!
-    if( pImpl->maDocTopics.empty() )
+    if (!pImpl->pDdeService)
        return;

    // prevent double submit
@@ -521,19 +522,22 @@ DdeData* SfxDdeDocTopic_Impl::Get(SotClipboardFormatId nFormat)

 bool SfxDdeDocTopic_Impl::Put( const DdeData* pData )
 {
-    aSeq = css::uno::Sequence< sal_Int8 >(
-                            static_cast<sal_Int8 const *>(pData->getData()), pData->getSize() );
-    bool bRet;
-    if( aSeq.getLength() )
+    if (pData->getSize())
    {
        css::uno::Any aValue;
-        aValue <<= aSeq;
-        OUString sMimeType( SotExchange::GetFormatMimeType( pData->GetFormat() ));
-        bRet = pSh->DdeSetData( GetCurItem(), sMimeType, aValue );
+        if (pData->GetFormat() == SotClipboardFormatId::STRING)
+        {
+            aValue <<= OUString(reinterpret_cast<const sal_Unicode*>(pData->getData()));
+        }
+        else
+        {
+            aValue <<= css::uno::Sequence(static_cast<sal_Int8 const*>(pData->getData()),
+                                          pData->getSize());
+        }
+        return pSh->DdeSetData(GetCurItem(), SotExchange::GetFormatMimeType(pData->GetFormat()),
+                               aValue);
    }
-    else
-        bRet = false;
-    return bRet;
+    return false;
 }

 bool SfxDdeDocTopic_Impl::Execute( const OUString* pStr )
@@ -161,9 +161,17 @@ static DdeTopic* FindTopic( const OUString & rLinkName, sal_uInt16* pItemStt )

            std::vector<DdeTopic*>& rTopics = elem->GetTopics();

-            for (auto const& topic : rTopics)
-                if( topic->GetName() == sTopic )
-                    return topic;
+            auto cfName = [&sTopic](const DdeTopic* p) { return p->GetName() == sTopic; };
+            auto it = std::find_if(rTopics.begin(), rTopics.end(), cfName);
+            if (it == rTopics.end())
+            {
+                // Topic not found?
+                // then we try once to create it
+                if (elem->MakeTopic(sTopic))
+                    it = std::find_if(rTopics.begin(), rTopics.end(), cfName);
+            }
+            if (it != rTopics.end())
+                return *it;
            break;
        }
    }
@@ -26,30 +26,15 @@
 #include <osl/thread.h>
 #include <comphelper/solarmutex.hxx>

-namespace {
+#include <com/sun/star/awt/Toolkit.hpp>
+#include <com/sun/star/awt/XToolkitExperimental.hpp>

-DdeInstData * theDdeInstData;
-
-}
-
-DdeInstData* ImpGetInstData()
+DdeInstData& ImpGetInstData()
 {
+    static DdeInstData theDdeInstData;
    return theDdeInstData;
 }

-DdeInstData* ImpInitInstData()
-{
-    theDdeInstData = new DdeInstData;
-    return theDdeInstData;
-}
-
-void ImpDeinitInstData()
-{
-    delete theDdeInstData;
-    theDdeInstData = nullptr;
-}
-
-
 struct DdeImp
 {
    HCONV   hConv;
@@ -64,8 +49,7 @@ HDDEDATA CALLBACK DdeInternal::CliCallback( UINT nCode, UINT nCbType,
    const std::vector<DdeConnection*> &rAll = DdeConnection::GetConnections();
    DdeConnection*      self = nullptr;

-    DdeInstData* pInst = ImpGetInstData();
-    assert(pInst);
+    const DdeInstData& rInst = ImpGetInstData();

    for ( size_t i = 0; i < rAll.size(); ++i)
    {
@@ -97,7 +81,7 @@ HDDEDATA CALLBACK DdeInternal::CliCallback( UINT nCode, UINT nCbType,
                self->pImp->hConv = DdeReconnect( hConv );
                self->pImp->nStatus = self->pImp->hConv
                    ? DMLERR_NO_ERROR
-                    : DdeGetLastError( pInst->hDdeInstCli );
+                    : DdeGetLastError( rInst.hDdeInstCli );
                iter = self->aTransactions.end();
                nRet = nullptr;
                bFound = true;
@@ -144,17 +128,15 @@ DdeConnection::DdeConnection( const OUString& rService, const OUString& rTopic )
    pImp->nStatus  = DMLERR_NO_ERROR;
    pImp->hConv    = nullptr;

-    DdeInstData* pInst = ImpGetInstData();
-    if( !pInst )
-        pInst = ImpInitInstData();
-    pInst->nRefCount++;
-    pInst->nInstanceCli++;
-    if ( !pInst->hDdeInstCli )
+    DdeInstData& rInst = ImpGetInstData();
+    rInst.nRefCount++;
+    rInst.nInstanceCli++;
+    if ( !rInst.hDdeInstCli )
    {
        pImp->nStatus = DMLERR_SYS_ERROR;
        if ( !officecfg::Office::Common::Security::Scripting::DisableActiveContent::get() )
        {
-            pImp->nStatus = DdeInitializeW( &pInst->hDdeInstCli,
+            pImp->nStatus = DdeInitializeW( &rInst.hDdeInstCli,
                                            DdeInternal::CliCallback,
                                            APPCLASS_STANDARD | APPCMD_CLIENTONLY |
                                            CBF_FAIL_ALLSVRXACTIONS |
@@ -163,45 +145,50 @@ DdeConnection::DdeConnection( const OUString& rService, const OUString& rTopic )
        }
    }

-    pService = new DdeString( pInst->hDdeInstCli, rService );
-    pTopic   = new DdeString( pInst->hDdeInstCli, rTopic );
+    pService = new DdeString( rInst.hDdeInstCli, rService );
+    pTopic   = new DdeString( rInst.hDdeInstCli, rTopic );

    if ( pImp->nStatus == DMLERR_NO_ERROR )
    {
-        pImp->hConv = DdeConnect( pInst->hDdeInstCli,pService->getHSZ(),pTopic->getHSZ(), nullptr);
+        pImp->hConv = DdeConnect( rInst.hDdeInstCli,pService->getHSZ(),pTopic->getHSZ(), nullptr);
        if( !pImp->hConv )
-            pImp->nStatus = DdeGetLastError( pInst->hDdeInstCli );
+            pImp->nStatus = DdeGetLastError( rInst.hDdeInstCli );
    }

-    pInst->aConnections.push_back( this );
+    rInst.aConnections.push_back( this );
 }

 DdeConnection::~DdeConnection()
 {
    if ( pImp->hConv )
+    {
        DdeDisconnect( pImp->hConv );
+        // DdeDisconnect sends a XTYP_DISCONNECT transaction to server. DdeUninitialize below will
+        // wait until it's finished. If server happens to be this app, it would deadlock unless we
+        // allow to process messages here:
+        auto xTk = css::awt::Toolkit::create(comphelper::getProcessComponentContext());
+        if (auto xTkExperimental = xTk.query<css::awt::XToolkitExperimental>())
+            xTkExperimental->processEventsToIdle();
+    }

    delete pService;
    delete pTopic;

-    DdeInstData* pInst = ImpGetInstData();
-    assert(pInst);
+    DdeInstData& rInst = ImpGetInstData();

-    std::vector<DdeConnection*>::iterator it(std::find(pInst->aConnections.begin(),
-                                                        pInst->aConnections.end(),
+    std::vector<DdeConnection*>::iterator it(std::find(rInst.aConnections.begin(),
+                                                        rInst.aConnections.end(),
                                                        this));
-    if (it != pInst->aConnections.end())
-        pInst->aConnections.erase(it);
+    if (it != rInst.aConnections.end())
+        rInst.aConnections.erase(it);

-    pInst->nInstanceCli--;
-    pInst->nRefCount--;
-    if ( !pInst->nInstanceCli && pInst->hDdeInstCli )
+    rInst.nInstanceCli--;
+    rInst.nRefCount--;
+    if ( !rInst.nInstanceCli && rInst.hDdeInstCli )
    {
-        if( DdeUninitialize( pInst->hDdeInstCli ) )
+        if( DdeUninitialize( rInst.hDdeInstCli ) )
        {
-            pInst->hDdeInstCli = 0;
-            if( pInst->nRefCount == 0 )
-                ImpDeinitInstData();
+            rInst.hDdeInstCli = 0;
        }
    }
 }
@@ -214,9 +201,9 @@ bool DdeConnection::IsConnected()
        return true;
    else
    {
-        DdeInstData* pInst = ImpGetInstData();
+        const DdeInstData& rInst = ImpGetInstData();
        pImp->hConv = DdeReconnect( pImp->hConv );
-        pImp->nStatus = pImp->hConv ? DMLERR_NO_ERROR : DdeGetLastError( pInst->hDdeInstCli );
+        pImp->nStatus = pImp->hConv ? DMLERR_NO_ERROR : DdeGetLastError( rInst.hDdeInstCli );
        return pImp->nStatus == DMLERR_NO_ERROR;
    }
 }
@@ -233,17 +220,14 @@ OUString DdeConnection::GetServiceName() const

 const std::vector<DdeConnection*>& DdeConnection::GetConnections()
 {
-    DdeInstData* pInst = ImpGetInstData();
-    assert(pInst);
-    return pInst->aConnections;
+    return ImpGetInstData().aConnections;
 }

 DdeTransaction::DdeTransaction( DdeConnection& d, const OUString& rItemName,
                                tools::Long n )
    : rDde( d )
 {
-    DdeInstData* pInst = ImpGetInstData();
-    pName = new DdeString( pInst->hDdeInstCli, rItemName );
+    pName = new DdeString( ImpGetInstData().hDdeInstCli, rItemName );
    nTime = n;
    nId   = 0;
    nType = 0;
@@ -256,8 +240,7 @@ DdeTransaction::~DdeTransaction()
 {
    if ( nId && rDde.pImp->hConv )
    {
-        DdeInstData* pInst = ImpGetInstData();
-        DdeAbandonTransaction( pInst->hDdeInstCli, rDde.pImp->hConv, nId );
+        DdeAbandonTransaction( ImpGetInstData().hDdeInstCli, rDde.pImp->hConv, nId );
    }

    delete pName;
@@ -271,7 +254,7 @@ void DdeTransaction::Execute()
    DWORD   nData = static_cast<DWORD>(aDdeData.getSize());
    SotClipboardFormatId nIntFmt = aDdeData.xImp->nFmt;
    UINT    nExtFmt  = DdeData::GetExternalFormat( nIntFmt );
-    DdeInstData* pInst = ImpGetInstData();
+    const DdeInstData& rInst = ImpGetInstData();

    if ( nType == XTYP_EXECUTE )
        hItem = nullptr;
@@ -287,7 +270,7 @@ void DdeTransaction::Execute()
                                               hItem, nExtFmt, static_cast<UINT>(nType),
                                               static_cast<DWORD>(nTime), nullptr );

-        rDde.pImp->nStatus = DdeGetLastError( pInst->hDdeInstCli );
+        rDde.pImp->nStatus = DdeGetLastError( rInst.hDdeInstCli );
        if( hData && nType == XTYP_REQUEST )
        {
            {
@@ -303,7 +286,7 @@ void DdeTransaction::Execute()
    else
    {
        if ( nId && rDde.pImp->hConv )
-            DdeAbandonTransaction( pInst->hDdeInstCli, rDde.pImp->hConv, nId);
+            DdeAbandonTransaction( rInst.hDdeInstCli, rDde.pImp->hConv, nId);
        nId = 0;
        bBusy = true;
        DWORD result;
@@ -313,7 +296,7 @@ void DdeTransaction::Execute()
                                            &result );
        nId = result;
        rDde.pImp->nStatus = hRet ? DMLERR_NO_ERROR
-                                  : DdeGetLastError( pInst->hDdeInstCli );
+                                  : DdeGetLastError( rInst.hDdeInstCli );
    }
 }

@@ -53,7 +53,7 @@ DdeData::DdeData( const OUString& s )
    xImp.reset(new DdeDataImp);
    xImp->hData = nullptr;
    xImp->pData = s.getStr();
-    xImp->nData = s.getLength()+1;
+    xImp->nData = (s.getLength() + 1) * sizeof(sal_Unicode);
    xImp->nFmt = SotClipboardFormatId::STRING;
 }

@@ -108,9 +108,7 @@ public:
    DdeInstData& operator=(const DdeInstData&) = delete;
 };

-DdeInstData* ImpGetInstData();
-DdeInstData* ImpInitInstData();
-void ImpDeinitInstData();
+DdeInstData& ImpGetInstData();

 #endif // INCLUDED_SVL_SOURCE_SVDDE_DDEIMP_HXX

@@ -50,8 +50,7 @@ HDDEDATA CALLBACK DdeInternal::SvrCallback(
            UINT nCode, UINT nCbType, HCONV hConv, HSZ hText1, HSZ hText2,
            HDDEDATA hData, ULONG_PTR, ULONG_PTR )
 {
-    DdeInstData* pInst = ImpGetInstData();
-    assert(pInst);
+    const DdeInstData& rInst = ImpGetInstData();

    switch( nCode )
    {
@@ -61,7 +60,7 @@ HDDEDATA CALLBACK DdeInternal::SvrCallback(

            WCHAR chTopicBuf[256];
            if( hText1 )
-                DdeQueryStringW( pInst->hDdeInstSvr, hText1, chTopicBuf,
+                DdeQueryStringW( rInst.hDdeInstSvr, hText1, chTopicBuf,
                                SAL_N_ELEMENTS(chTopicBuf), CP_WINUNICODE );

            for (auto& pService : DdeService::GetServices())
@@ -79,7 +78,7 @@ HDDEDATA CALLBACK DdeInternal::SvrCallback(
                    if (hText1 && s != o3tl::toU(chTopicBuf))
                        continue;

-                    DdeString aDStr(pInst->hDdeInstSvr, s);
+                    DdeString aDStr(rInst.hDdeInstSvr, s);
                    if (auto pTopic = FindTopic(*pService, aDStr.getHSZ()))
                    {
                        auto& pair = aPairs.emplace_back();
@@ -94,7 +93,7 @@ HDDEDATA CALLBACK DdeInternal::SvrCallback(
            aPairs.emplace_back(); // trailing zero

            HDDEDATA h = DdeCreateDataHandle(
-                            pInst->hDdeInstSvr,
+                            rInst.hDdeInstSvr,
                            reinterpret_cast<LPBYTE>(aPairs.data()),
                            sizeof(HSZPAIR) * aPairs.size(),
                            0, nullptr, nCbType, 0);
@@ -201,7 +200,7 @@ HDDEDATA CALLBACK DdeInternal::SvrCallback(

            if ( pData )
            {
-                return DdeCreateDataHandle( pInst->hDdeInstSvr,
+                return DdeCreateDataHandle( rInst.hDdeInstSvr,
                                            static_cast<LPBYTE>(const_cast<void *>(pData->xImp->pData)),
                                            pData->xImp->nData,
                                            0, hText2,
@@ -308,8 +307,22 @@ DdeTopic* DdeInternal::FindTopic( DdeService& rService, HSZ hTopic )
 {
    std::vector<DdeTopic*> &rTopics = rService.aTopics;

-    auto iter = std::find_if(rTopics.begin(), rTopics.end(),
-        [&hTopic](const DdeTopic* pTopic) { return *pTopic->pName == hTopic; });
+    auto cfName = [&hTopic](const DdeTopic* pTopic) { return *pTopic->pName == hTopic; };
+    auto iter = std::find_if(rTopics.begin(), rTopics.end(), cfName);
+    if (iter == rTopics.end())
+    {
+        // Let's query our subclass
+        const DdeInstData& rInst = ImpGetInstData();
+        if (DWORD sz = DdeQueryStringW(rInst.hDdeInstSvr, hTopic, nullptr, 0, CP_WINUNICODE))
+        {
+            auto chBuf = std::make_unique<WCHAR[]>(sz + 1);
+            sz = DdeQueryStringW(rInst.hDdeInstSvr, hTopic, chBuf.get(), sz + 1, CP_WINUNICODE);
+            if (sz && rService.MakeTopic(OUString(o3tl::toU(chBuf.get()), sz)))
+            {
+                iter = std::find_if(rTopics.begin(), rTopics.end(), cfName);
+            }
+        }
+    }
    if (iter != rTopics.end())
        return *iter;

@@ -320,8 +333,7 @@ DdeItem* DdeInternal::FindItem( DdeTopic& rTopic, HSZ hItem )
 {
    std::vector<DdeItem*>::iterator iter;
    std::vector<DdeItem*> &rItems = rTopic.aItems;
-    DdeInstData* pInst = ImpGetInstData();
-    assert(pInst);
+    const DdeInstData& rInst = ImpGetInstData();
    bool bContinue = false;

    do
@@ -336,7 +348,7 @@ DdeItem* DdeInternal::FindItem( DdeTopic& rTopic, HSZ hItem )

        // Let's query our subclass
        WCHAR chBuf[250];
-        DdeQueryStringW(pInst->hDdeInstSvr,hItem,chBuf,SAL_N_ELEMENTS(chBuf),CP_WINUNICODE );
+        DdeQueryStringW(rInst.hDdeInstSvr,hItem,chBuf,SAL_N_ELEMENTS(chBuf),CP_WINUNICODE );
        bContinue = rTopic.MakeItem( OUString(o3tl::toU(chBuf)) );
        // We need to search again
    }
@@ -347,36 +359,34 @@ DdeItem* DdeInternal::FindItem( DdeTopic& rTopic, HSZ hItem )

 DdeService::DdeService( const OUString& rService )
 {
-    DdeInstData* pInst = ImpGetInstData();
-    if( !pInst )
-        pInst = ImpInitInstData();
-    pInst->nRefCount++;
-    pInst->nInstanceSvr++;
+    DdeInstData& rInst = ImpGetInstData();
+    rInst.nRefCount++;
+    rInst.nInstanceSvr++;

-    if ( !pInst->hDdeInstSvr )
+    if ( !rInst.hDdeInstSvr )
    {
        nStatus = DMLERR_SYS_ERROR;
        if ( !officecfg::Office::Common::Security::Scripting::DisableActiveContent::get() )
        {
            nStatus = sal::static_int_cast< short >(
-                DdeInitializeW( &pInst->hDdeInstSvr,
+                DdeInitializeW( &rInst.hDdeInstSvr,
                                DdeInternal::SvrCallback,
                                APPCLASS_STANDARD |
                                CBF_SKIP_REGISTRATIONS |
                                CBF_SKIP_UNREGISTRATIONS, 0 ) );
        }
-        pInst->pServicesSvr = new DdeServices;
+        rInst.pServicesSvr = new DdeServices;
    }
    else
        nStatus = DMLERR_NO_ERROR;

-    if ( pInst->pServicesSvr )
-        pInst->pServicesSvr->push_back( this );
+    if ( rInst.pServicesSvr )
+        rInst.pServicesSvr->push_back( this );

-    pName = new DdeString( pInst->hDdeInstSvr, rService );
+    pName = new DdeString( rInst.hDdeInstSvr, rService );
    if ( nStatus == DMLERR_NO_ERROR )
    {
-        if ( !DdeNameService( pInst->hDdeInstSvr, pName->getHSZ(), nullptr,
+        if ( !DdeNameService( rInst.hDdeInstSvr, pName->getHSZ(), nullptr,
                              DNS_REGISTER | DNS_FILTEROFF ) )
        {
            nStatus = DMLERR_SYS_ERROR;
@@ -394,25 +404,22 @@ DdeService::DdeService( const OUString& rService )

 DdeService::~DdeService()
 {
-    DdeInstData* pInst = ImpGetInstData();
-    assert(pInst);
-    if ( pInst->pServicesSvr )
-        std::erase(*pInst->pServicesSvr, this);
+    DdeInstData& rInst = ImpGetInstData();
+    if ( rInst.pServicesSvr )
+        std::erase(*rInst.pServicesSvr, this);

    delete pSysTopic;
    delete pName;

-    pInst->nInstanceSvr--;
-    pInst->nRefCount--;
-    if ( !pInst->nInstanceSvr && pInst->hDdeInstSvr )
+    rInst.nInstanceSvr--;
+    rInst.nRefCount--;
+    if ( !rInst.nInstanceSvr && rInst.hDdeInstSvr )
    {
-        if( DdeUninitialize( pInst->hDdeInstSvr ) )
+        if( DdeUninitialize( rInst.hDdeInstSvr ) )
        {
-            pInst->hDdeInstSvr = 0;
-            delete pInst->pServicesSvr;
-            pInst->pServicesSvr = nullptr;
-            if( pInst->nRefCount == 0)
-                ImpDeinitInstData();
+            rInst.hDdeInstSvr = 0;
+            delete rInst.pServicesSvr;
+            rInst.pServicesSvr = nullptr;
        }
    }
 }
@@ -424,9 +431,7 @@ OUString DdeService::GetName() const

 DdeServices& DdeService::GetServices()
 {
-    DdeInstData* pInst = ImpGetInstData();
-    assert(pInst);
-    return *(pInst->pServicesSvr);
+    return *(ImpGetInstData().pServicesSvr);
 }

 void DdeService::AddTopic( const DdeTopic& rTopic )
@@ -481,9 +486,7 @@ void DdeService::RemoveFormat(SotClipboardFormatId nFmt)

 DdeTopic::DdeTopic( const OUString& rName )
 {
-    DdeInstData* pInst = ImpGetInstData();
-    assert(pInst);
-    pName = new DdeString( pInst->hDdeInstSvr, rName );
+    pName = new DdeString( ImpGetInstData().hDdeInstSvr, rName );
 }

 DdeTopic::~DdeTopic()
@@ -544,12 +547,10 @@ void DdeTopic::RemoveItem( const DdeItem& r )

 void DdeTopic::NotifyClient( const OUString& rItem )
 {
-    DdeInstData* pInst = ImpGetInstData();
-    assert(pInst);
    auto iter = std::find_if(aItems.begin(), aItems.end(),
        [&rItem](const DdeItem* pItem) { return pItem->GetName().equals(rItem) && pItem->pImpData; });
    if (iter != aItems.end())
-        DdePostAdvise( pInst->hDdeInstSvr, pName->getHSZ(), (*iter)->pName->getHSZ() );
+        DdePostAdvise( ImpGetInstData().hDdeInstSvr, pName->getHSZ(), (*iter)->pName->getHSZ() );
 }

 void DdeInternal::DisconnectTopic(DdeTopic & rTopic, HCONV nId)
@@ -581,33 +582,21 @@ bool DdeTopic::StartAdviseLoop()
 }

 DdeItem::DdeItem( const sal_Unicode* p )
+    : DdeItem(OUString(p))
 {
-    DdeInstData* pInst = ImpGetInstData();
-    assert(pInst);
-    pName = new DdeString( pInst->hDdeInstSvr, OUString(p) );
-    nType = DDEITEM;
-    pMyTopic = nullptr;
-    pImpData = nullptr;
 }

 DdeItem::DdeItem( const OUString& r)
 {
-    DdeInstData* pInst = ImpGetInstData();
-    assert(pInst);
-    pName = new DdeString( pInst->hDdeInstSvr, r );
+    pName = new DdeString( ImpGetInstData().hDdeInstSvr, r );
    nType = DDEITEM;
    pMyTopic = nullptr;
    pImpData = nullptr;
 }

 DdeItem::DdeItem( const DdeItem& r)
+    : DdeItem(r.pName->toOUString())
 {
-    DdeInstData* pInst = ImpGetInstData();
-    assert(pInst);
-    pName = new DdeString( pInst->hDdeInstSvr, r.pName->toOUString() );
-    nType = DDEITEM;
-    pMyTopic = nullptr;
-    pImpData = nullptr;
 }

 DdeItem::~DdeItem()
@@ -627,9 +616,7 @@ void DdeItem::NotifyClient()
 {
    if( pMyTopic && pImpData )
    {
-        DdeInstData* pInst = ImpGetInstData();
-        assert(pInst);
-        DdePostAdvise( pInst->hDdeInstSvr, pMyTopic->pName->getHSZ(), pName->getHSZ() );
+        DdePostAdvise( ImpGetInstData().hDdeInstSvr, pMyTopic->pName->getHSZ(), pName->getHSZ() );
    }
 }