[ros-diffs] [reactos] 02/02: [EXPLORER][BROWSEUI][SHELL32][NETSHELL] Fix wrong usage of CComPtr

[Mark Jansen]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=cd2d284142c1fcaefc975…
commit cd2d284142c1fcaefc9759412c51c6d2fb2ca461
Author:     Mark Jansen <mark.jansen(a)reactos.org>
AuthorDate: Sun Sep 18 20:59:00 2022 +0200
Commit:     Mark Jansen <mark.jansen(a)reactos.org>
CommitDate: Sun Oct 2 00:38:23 2022 +0200
    [EXPLORER][BROWSEUI][SHELL32][NETSHELL] Fix wrong usage of CComPtr
---
 base/shell/explorer/startctxmnu.cpp                |  5 +----
 base/shell/explorer/tbsite.cpp                     |  2 +-
 base/shell/explorer/traywnd.cpp                    |  1 -
 dll/shellext/netshell/lanstatusui.cpp              |  2 +-
 dll/win32/browseui/CAutoComplete.cpp               |  6 ------
 dll/win32/browseui/desktopipc.cpp                  | 22 ++++++++--------------
 dll/win32/browseui/internettoolbar.cpp             |  2 --
 dll/win32/browseui/shellbrowser.cpp                |  2 +-
 dll/win32/shell32/CFolderItems.cpp                 |  2 +-
 dll/win32/shell32/shelldesktop/CDesktopBrowser.cpp |  4 ++--
 dll/win32/shell32/shellmenu/CMenuBand.cpp          |  6 +++---
 sdk/include/reactos/browseui_undoc.h               |  4 ++--
 sdk/include/reactos/shellutils.h                   |  6 +++---
 13 files changed, 23 insertions(+), 41 deletions(-)
diff --git a/base/shell/explorer/startctxmnu.cpp b/base/shell/explorer/startctxmnu.cpp
index 935c8d7773d..50fea3ab6db 100644
--- a/base/shell/explorer/startctxmnu.cpp
+++ b/base/shell/explorer/startctxmnu.cpp
@@ -242,8 +242,5 @@ public:
 HRESULT CStartMenuBtnCtxMenu_CreateInstance(ITrayWindow * m_TrayWnd, IN HWND m_Owner,
IContextMenu ** ppCtxMenu)
 {
-    CStartMenuBtnCtxMenu * mnu = new CComObject<CStartMenuBtnCtxMenu>();
-    mnu->Initialize(m_TrayWnd, m_Owner);
-    *ppCtxMenu = mnu;
-    return S_OK;
+    return ShellObjectCreatorInit<CStartMenuBtnCtxMenu>(m_TrayWnd, m_Owner,
IID_PPV_ARG(IContextMenu, ppCtxMenu));
 }
diff --git a/base/shell/explorer/tbsite.cpp b/base/shell/explorer/tbsite.cpp
index 5ab31c76479..387cdb4d188 100644
--- a/base/shell/explorer/tbsite.cpp
+++ b/base/shell/explorer/tbsite.cpp
@@ -301,8 +301,8 @@ public:
         if (ppcm != NULL)
         {
-            m_ContextMenu->AddRef();
             *ppcm = m_ContextMenu;
+            (*ppcm)->AddRef();
         }
         /* Add the menu items */
diff --git a/base/shell/explorer/traywnd.cpp b/base/shell/explorer/traywnd.cpp
index b8437fe23cd..d3774451270 100644
--- a/base/shell/explorer/traywnd.cpp
+++ b/base/shell/explorer/traywnd.cpp
@@ -2943,7 +2943,6 @@ ChangePos:
             {
                 CComPtr<IContextMenu> ctxMenu;
                 CStartMenuBtnCtxMenu_CreateInstance(this, m_hWnd, &ctxMenu);
-                ctxMenu->AddRef();
                 TrackCtxMenu(ctxMenu, ppt, hWndExclude, m_Position == ABE_BOTTOM, this);
             }
         }
diff --git a/dll/shellext/netshell/lanstatusui.cpp b/dll/shellext/netshell/lanstatusui.cpp
index a6d3b96e744..c3be62dcad3 100644
--- a/dll/shellext/netshell/lanstatusui.cpp
+++ b/dll/shellext/netshell/lanstatusui.cpp
@@ -953,7 +953,7 @@ CLanStatus::InitializeNetTaskbarNotifications()
         pItem->uID = Index;
         pItem->pNext = NULL;
         pItem->pNet = pNetCon;
-        pNetCon->AddRef();
+        pItem->pNet->AddRef();
         hwndDlg = CreateDialogParamW(netshell_hInstance, MAKEINTRESOURCEW(IDD_STATUS),
NULL, LANStatusDlg, (LPARAM)pContext);
         if (!hwndDlg)
         {
diff --git a/dll/win32/browseui/CAutoComplete.cpp b/dll/win32/browseui/CAutoComplete.cpp
index 6ff30edcefc..f9929870085 100644
--- a/dll/win32/browseui/CAutoComplete.cpp
+++ b/dll/win32/browseui/CAutoComplete.cpp
@@ -1126,18 +1126,12 @@ CAutoComplete::Init(HWND hwndEdit, IUnknown *punkACL,
     ::GetWindowRect(m_hwndEdit, &m_rcEdit);
     // get an IEnumString
-    ATLASSERT(!m_pEnum);
     punkACL->QueryInterface(IID_IEnumString, (VOID **)&m_pEnum);
     TRACE("m_pEnum: %p\n", static_cast<void *>(m_pEnum));
-    if (m_pEnum)
-        m_pEnum->AddRef(); // hold not to be freed
     // get an IACList
-    ATLASSERT(!m_pACList);
     punkACL->QueryInterface(IID_IACList, (VOID **)&m_pACList);
     TRACE("m_pACList: %p\n", static_cast<void *>(m_pACList));
-    if (m_pACList)
-        m_pACList->AddRef(); // hold not to be freed
     UpdateDropDownState(); // create/hide the drop-down window if necessary
diff --git a/dll/win32/browseui/desktopipc.cpp b/dll/win32/browseui/desktopipc.cpp
index ba5d168a2a3..1e7cc68af6e 100644
--- a/dll/win32/browseui/desktopipc.cpp
+++ b/dll/win32/browseui/desktopipc.cpp
@@ -348,8 +348,8 @@ static HRESULT ExplorerMessageLoop(IEThreadParamBlock * parameters)
     BOOL Ret;
     // Tell the thread ref we are using it.
-    if (parameters && parameters->offsetF8)
-        parameters->offsetF8->AddRef();
+    if (parameters && parameters->pExplorerInstance)
+        parameters->pExplorerInstance->AddRef();
     /* Handle /e parameter */
      UINT wFlags = 0;
@@ -410,17 +410,11 @@ static HRESULT ExplorerMessageLoop(IEThreadParamBlock * parameters)
         }
     }
-    int nrc = browser->Release();
-    if (nrc > 0)
-    {
-        DbgPrint("WARNING: There are %d references to the CShellBrowser active or
leaked.\n", nrc);
-    }
-
-    browser.Detach();
+    ReleaseCComPtrExpectZero(browser);
     // Tell the thread ref we are not using it anymore.
-    if (parameters && parameters->offsetF8)
-        parameters->offsetF8->Release();
+    if (parameters && parameters->pExplorerInstance)
+        parameters->pExplorerInstance->Release();
     return hResult;
 }
@@ -519,8 +513,8 @@ extern "C" void WINAPI
SHDestroyIETHREADPARAM(IEThreadParamBlock *param)
         param->offset78->Release();
     if (param->offsetC != NULL)
         param->offsetC->Release();
-    if (param->offsetF8 != NULL)
-        param->offsetF8->Release();
+    if (param->pExplorerInstance != NULL)
+        param->pExplorerInstance->Release();
     LocalFree(param);
 }
@@ -563,7 +557,7 @@ extern "C" HRESULT WINAPI
SHOpenFolderWindow(PIE_THREAD_PARAM_BLOCK parameters)
     PIE_THREAD_PARAM_BLOCK paramsCopy = SHCloneIETHREADPARAM(parameters);
-    SHGetInstanceExplorer(&(paramsCopy->offsetF8));
+    SHGetInstanceExplorer(&(paramsCopy->pExplorerInstance));
     threadHandle = CreateThread(NULL, 0x10000, BrowserThreadProc, paramsCopy, 0,
&threadID);
     if (threadHandle != NULL)
     {
diff --git a/dll/win32/browseui/internettoolbar.cpp
b/dll/win32/browseui/internettoolbar.cpp
index bacc5110ade..8c71f4d1a3f 100644
--- a/dll/win32/browseui/internettoolbar.cpp
+++ b/dll/win32/browseui/internettoolbar.cpp
@@ -614,8 +614,6 @@ CInternetToolbar::CInternetToolbar()
     fMenuCallback = new CComObject<CMenuCallback>();
     fToolbarWindow = NULL;
     fAdviseCookie = 0;
-
-    fMenuCallback->AddRef();
 }
 CInternetToolbar::~CInternetToolbar()
diff --git a/dll/win32/browseui/shellbrowser.cpp b/dll/win32/browseui/shellbrowser.cpp
index 2704f2bdfca..b12fc20e82d 100644
--- a/dll/win32/browseui/shellbrowser.cpp
+++ b/dll/win32/browseui/shellbrowser.cpp
@@ -1121,7 +1121,7 @@ HRESULT CShellBrowser::GetBaseBar(bool vertical, REFIID riid, void
**theBaseBar)
         // we have to store our basebar into cache now
         *cache = newBaseBar;
-        newBaseBar->AddRef();
+        (*cache)->AddRef();
         // tell the new base bar about the shell browser
         hResult = IUnknown_SetSite(newBaseBar, static_cast<IDropTarget *>(this));
diff --git a/dll/win32/shell32/CFolderItems.cpp b/dll/win32/shell32/CFolderItems.cpp
index 5952120c80f..0ba76dc5273 100644
--- a/dll/win32/shell32/CFolderItems.cpp
+++ b/dll/win32/shell32/CFolderItems.cpp
@@ -38,7 +38,7 @@ HRESULT STDMETHODCALLTYPE CFolderItem::get_Parent(IDispatch **ppid)
     if (ppid)
     {
         *ppid = m_Folder;
-        m_Folder->AddRef();
+        (*ppid)->AddRef();
     }
     return E_NOTIMPL;
 }
diff --git a/dll/win32/shell32/shelldesktop/CDesktopBrowser.cpp
b/dll/win32/shell32/shelldesktop/CDesktopBrowser.cpp
index bfd205a7fdb..3e45650e9c6 100644
--- a/dll/win32/shell32/shelldesktop/CDesktopBrowser.cpp
+++ b/dll/win32/shell32/shelldesktop/CDesktopBrowser.cpp
@@ -321,8 +321,8 @@ HRESULT STDMETHODCALLTYPE
CDesktopBrowser::QueryActiveShellView(IShellView **pps
     if (ppshv == NULL)
         return E_POINTER;
     *ppshv = m_ShellView;
-    if (m_ShellView != NULL)
-        m_ShellView->AddRef();
+    if (*ppshv != NULL)
+        (*ppshv)->AddRef();
     return S_OK;
 }
diff --git a/dll/win32/shell32/shellmenu/CMenuBand.cpp
b/dll/win32/shell32/shellmenu/CMenuBand.cpp
index 42dec905690..d51dabbddf0 100644
--- a/dll/win32/shell32/shellmenu/CMenuBand.cpp
+++ b/dll/win32/shell32/shellmenu/CMenuBand.cpp
@@ -99,9 +99,9 @@ HRESULT STDMETHODCALLTYPE  CMenuBand::GetMenuInfo(
     if (ppsmc)
     {
-        if (m_psmc)
-            m_psmc->AddRef();
         *ppsmc = m_psmc;
+        if (*ppsmc)
+            (*ppsmc)->AddRef();
     }
     if (puId)
@@ -655,8 +655,8 @@ HRESULT STDMETHODCALLTYPE CMenuBand::GetClient(IUnknown **ppunkClient)
     if (m_subMenuChild)
     {
-        m_subMenuChild->AddRef();
         *ppunkClient = m_subMenuChild;
+        (*ppunkClient)->AddRef();
     }
     return S_OK;
diff --git a/sdk/include/reactos/browseui_undoc.h b/sdk/include/reactos/browseui_undoc.h
index bc0b455588f..3104a4dbd5a 100644
--- a/sdk/include/reactos/browseui_undoc.h
+++ b/sdk/include/reactos/browseui_undoc.h
@@ -60,7 +60,7 @@ typedef struct IEThreadParamBlock
     UCHAR gap108[24];
     DWORD dword120;
     DWORD dword124;
-    IUnknown*                       offsetF8; // 0x128 instance explorer
+    IUnknown*                       pExplorerInstance; // 0x128 instance explorer
     UCHAR byteflags_130;
 } IE_THREAD_PARAM_BLOCK, * PIE_THREAD_PARAM_BLOCK;
 #else
@@ -90,7 +90,7 @@ typedef struct IEThreadParamBlock
     char                            offsetA4[0xD8-0xA4];    // unknown contents --
0xA4..0xD8
     LONG                            offsetD8;
     char                            offsetDC[0xF8-0xDC];    // unknown contents --
0xDC..0xF8
-    IUnknown                      * offsetF8;        // instance explorer
+    IUnknown                      * pExplorerInstance;        // instance explorer
     LONG                            offsetFC;        // unknown contents
 } IE_THREAD_PARAM_BLOCK, *PIE_THREAD_PARAM_BLOCK;
 #endif
diff --git a/sdk/include/reactos/shellutils.h b/sdk/include/reactos/shellutils.h
index 2dcbd8037b0..7a3c7265059 100644
--- a/sdk/include/reactos/shellutils.h
+++ b/sdk/include/reactos/shellutils.h
@@ -226,7 +226,8 @@ void ReleaseCComPtrExpectZero(CComPtr<T>& cptr, BOOL
forceRelease = FALSE)
 {
     if (cptr.p != NULL)
     {
-        int nrc = cptr->Release();
+        T *raw = cptr.Detach();
+        int nrc = raw->Release();
         if (nrc > 0)
         {
             DbgPrint("WARNING: Unexpected RefCount > 0 (%d)!\n", nrc);
@@ -234,11 +235,10 @@ void ReleaseCComPtrExpectZero(CComPtr<T>& cptr, BOOL
forceRelease = FALSE)
             {
                 while (nrc > 0)
                 {
-                    nrc = cptr->Release();
+                    nrc = raw->Release();
                 }
             }
         }
-        cptr.Detach();
     }
 }