From b5c2c0b05ad288e933428d08f300fb024bdee6b9 Mon Sep 17 00:00:00 2001
From: Nikolay Sivov <nsivov@codeweavers.com>
Date: Fri, 5 Jun 2015 16:21:05 +0300
Subject: [PATCH] shlwapi: Fix error handling in IUnknown_GetClassID
 (Coverity).

---
 dlls/shlwapi/ordinal.c       | 31 ++++++++-------
 dlls/shlwapi/tests/ordinal.c | 92 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 110 insertions(+), 13 deletions(-)

diff --git a/dlls/shlwapi/ordinal.c b/dlls/shlwapi/ordinal.c
index 3da25b49c82..68b66e360f6 100644
--- a/dlls/shlwapi/ordinal.c
+++ b/dlls/shlwapi/ordinal.c
@@ -1384,7 +1384,7 @@ HRESULT WINAPI IUnknown_SetSite(
  *
  * PARAMS
  *  lpUnknown [I] Object supporting the IPersist interface
- *  lpClassId [O] Destination for Class Id
+ *  clsid     [O] Destination for Class Id
  *
  * RETURNS
  *  Success: S_OK. lpClassId contains the Class Id requested.
@@ -1392,23 +1392,28 @@ HRESULT WINAPI IUnknown_SetSite(
  *           E_NOINTERFACE If lpUnknown does not support IPersist,
  *           Or an HRESULT error code.
  */
-HRESULT WINAPI IUnknown_GetClassID(IUnknown *lpUnknown, CLSID* lpClassId)
+HRESULT WINAPI IUnknown_GetClassID(IUnknown *lpUnknown, CLSID *clsid)
 {
-  IPersist* lpPersist;
-  HRESULT hRet = E_FAIL;
+    IPersist *persist;
+    HRESULT hr;
 
-  TRACE("(%p,%s)\n", lpUnknown, debugstr_guid(lpClassId));
+    TRACE("(%p, %p)\n", lpUnknown, clsid);
 
-  if (lpUnknown)
-  {
-    hRet = IUnknown_QueryInterface(lpUnknown,&IID_IPersist,(void**)&lpPersist);
-    if (SUCCEEDED(hRet))
+    if (!lpUnknown)
     {
-      IPersist_GetClassID(lpPersist, lpClassId);
-      IPersist_Release(lpPersist);
+        memset(clsid, 0, sizeof(*clsid));
+        return E_FAIL;
     }
-  }
-  return hRet;
+
+    hr = IUnknown_QueryInterface(lpUnknown, &IID_IPersist, (void**)&persist);
+    if (hr != S_OK)
+        hr = IUnknown_QueryInterface(lpUnknown, &IID_IPersistFolder, (void**)&persist);
+        if (hr != S_OK)
+            return hr;
+
+    hr = IPersist_GetClassID(persist, clsid);
+    IPersist_Release(persist);
+    return hr;
 }
 
 /*************************************************************************
diff --git a/dlls/shlwapi/tests/ordinal.c b/dlls/shlwapi/tests/ordinal.c
index d8a4f680321..9225219a9d5 100644
--- a/dlls/shlwapi/tests/ordinal.c
+++ b/dlls/shlwapi/tests/ordinal.c
@@ -69,6 +69,7 @@ static HRESULT (WINAPI *pSKSetValueW)(DWORD, LPCWSTR, LPCWSTR, DWORD, void*, DWO
 static HRESULT (WINAPI *pSKDeleteValueW)(DWORD, LPCWSTR, LPCWSTR);
 static HRESULT (WINAPI *pSKAllocValueW)(DWORD, LPCWSTR, LPCWSTR, DWORD*, void**, DWORD*);
 static HWND    (WINAPI *pSHSetParentHwnd)(HWND, HWND);
+static HRESULT (WINAPI *pIUnknown_GetClassID)(IUnknown*, CLSID*);
 
 static HMODULE hmlang;
 static HRESULT (WINAPI *pLcidToRfc1766A)(LCID, LPSTR, INT);
@@ -3047,6 +3048,7 @@ static void init_pointers(void)
     MAKEFUNC(SHSetWindowBits, 165);
     MAKEFUNC(SHSetParentHwnd, 167);
     MAKEFUNC(ConnectToConnectionPoint, 168);
+    MAKEFUNC(IUnknown_GetClassID, 175);
     MAKEFUNC(SHSearchMapInt, 198);
     MAKEFUNC(SHCreateWorkerWindowA, 257);
     MAKEFUNC(GUIDFromStringA, 269);
@@ -3152,6 +3154,95 @@ static void test_SHSetParentHwnd(void)
     DestroyWindow(hwnd2);
 }
 
+static HRESULT WINAPI testpersist_QI(IPersist *iface, REFIID riid, void **obj)
+{
+    if (IsEqualIID(riid, &IID_IUnknown) || IsEqualIID(riid, &IID_IPersist)) {
+        *obj = iface;
+        IPersist_AddRef(iface);
+        return S_OK;
+    }
+
+    *obj = NULL;
+    return E_NOINTERFACE;
+}
+
+static HRESULT WINAPI testpersist_QI2(IPersist *iface, REFIID riid, void **obj)
+{
+    if (IsEqualIID(riid, &IID_IUnknown) || IsEqualIID(riid, &IID_IPersistFolder)) {
+        *obj = iface;
+        IPersist_AddRef(iface);
+        return S_OK;
+    }
+
+    *obj = NULL;
+    return E_NOINTERFACE;
+}
+
+static ULONG WINAPI testpersist_AddRef(IPersist *iface)
+{
+    return 2;
+}
+
+static ULONG WINAPI testpersist_Release(IPersist *iface)
+{
+    return 1;
+}
+
+static HRESULT WINAPI testpersist_GetClassID(IPersist *iface, CLSID *clsid)
+{
+    memset(clsid, 0xab, sizeof(*clsid));
+    return 0x8fff2222;
+}
+
+static IPersistVtbl testpersistvtbl = {
+    testpersist_QI,
+    testpersist_AddRef,
+    testpersist_Release,
+    testpersist_GetClassID
+};
+
+static IPersistVtbl testpersist2vtbl = {
+    testpersist_QI2,
+    testpersist_AddRef,
+    testpersist_Release,
+    testpersist_GetClassID
+};
+
+static IPersist testpersist = { &testpersistvtbl };
+static IPersist testpersist2 = { &testpersist2vtbl };
+
+static void test_IUnknown_GetClassID(void)
+{
+    CLSID clsid, clsid2, clsid3;
+    HRESULT hr;
+
+if (0) /* crashes on native systems */
+    hr = pIUnknown_GetClassID(NULL, NULL);
+
+    memset(&clsid, 0xcc, sizeof(clsid));
+    memset(&clsid3, 0xcc, sizeof(clsid3));
+    hr = pIUnknown_GetClassID(NULL, &clsid);
+    ok(hr == E_FAIL, "got 0x%08x\n", hr);
+    ok(IsEqualCLSID(&clsid, &CLSID_NULL) || broken(IsEqualCLSID(&clsid, &clsid3)) /* win2k, winxp, win2k3 */,
+        "got wrong clsid %s\n", wine_dbgstr_guid(&clsid));
+
+    memset(&clsid, 0xcc, sizeof(clsid));
+    memset(&clsid2, 0xab, sizeof(clsid2));
+    hr = pIUnknown_GetClassID((IUnknown*)&testpersist, &clsid);
+    ok(hr == 0x8fff2222, "got 0x%08x\n", hr);
+    ok(IsEqualCLSID(&clsid, &clsid2) || broken(IsEqualCLSID(&clsid, &clsid3)) /* win2k3 */,
+        "got wrong clsid %s\n", wine_dbgstr_guid(&clsid));
+
+    /* IPersistFolder is also supported */
+    memset(&clsid, 0xcc, sizeof(clsid));
+    memset(&clsid2, 0xab, sizeof(clsid2));
+    memset(&clsid3, 0xcc, sizeof(clsid3));
+    hr = pIUnknown_GetClassID((IUnknown*)&testpersist2, &clsid);
+    ok(hr == 0x8fff2222, "got 0x%08x\n", hr);
+    ok(IsEqualCLSID(&clsid, &clsid2) || broken(IsEqualCLSID(&clsid, &clsid3)) /* win2k3 */,
+        "got wrong clsid %s\n", wine_dbgstr_guid(&clsid));
+}
+
 START_TEST(ordinal)
 {
     char **argv;
@@ -3206,6 +3297,7 @@ START_TEST(ordinal)
     test_SHSetIniString();
     test_SHGetShellKey();
     test_SHSetParentHwnd();
+    test_IUnknown_GetClassID();
 
     FreeLibrary(hshell32);
     FreeLibrary(hmlang);
-- 
2.11.4.GIT