Nikolay Sivov : oleaut32: Fix VariantCopy() for VT_RECORD variants.

[Alexandre Julliard]

Module: wine
Branch: master
Commit: e610713012cb02648202a9568fc0d3febb5f5307
URL:    http://source.winehq.org/git/wine.git/?a=commit;h=e610713012cb02648202a9568f...

Author: Nikolay Sivov <nsivov(a)codeweavers.com>
Date:   Thu Feb 20 19:12:57 2014 +0400

oleaut32: Fix VariantCopy() for VT_RECORD variants.

---

 dlls/oleaut32/tests/vartest.c |   50 ++++++++++++++++++++++----
 dlls/oleaut32/variant.c       |   77 +++++++++++++++++++----------------------
 2 files changed, 79 insertions(+), 48 deletions(-)

diff --git a/dlls/oleaut32/tests/vartest.c b/dlls/oleaut32/tests/vartest.c
index 2658025..2f82d9b 100644
--- a/dlls/oleaut32/tests/vartest.c
+++ b/dlls/oleaut32/tests/vartest.c
@@ -102,6 +102,8 @@ typedef struct IRecordInfoImpl
     IRecordInfo IRecordInfo_iface;
     LONG ref;
     unsigned int recordclear;
+    unsigned int getsize;
+    unsigned int recordcopy;
     struct __tagBRECORD *rec;
 } IRecordInfoImpl;
 
@@ -152,15 +154,16 @@ static HRESULT WINAPI RecordInfo_RecordClear(IRecordInfo *iface, void *data)
 {
     IRecordInfoImpl* This = impl_from_IRecordInfo(iface);
     This->recordclear++;
-    ok(data == (void*)0xdeadbeef, "got %p\n", data);
     This->rec->pvRecord = NULL;
     return S_OK;
 }
 
-static HRESULT WINAPI RecordInfo_RecordCopy(IRecordInfo *iface, void *pvExisting, void *pvNew)
+static HRESULT WINAPI RecordInfo_RecordCopy(IRecordInfo *iface, void *src, void *dest)
 {
-    ok(0, "unexpected call\n");
-    return E_NOTIMPL;
+    IRecordInfoImpl* This = impl_from_IRecordInfo(iface);
+    This->recordcopy++;
+    ok(src == (void*)0xdeadbeef, "wrong src pointer %p\n", src);
+    return S_OK;
 }
 
 static HRESULT WINAPI RecordInfo_GetGuid(IRecordInfo *iface, GUID *pguid)
@@ -177,8 +180,10 @@ static HRESULT WINAPI RecordInfo_GetName(IRecordInfo *iface, BSTR *pbstrName)
 
 static HRESULT WINAPI RecordInfo_GetSize(IRecordInfo *iface, ULONG* size)
 {
-    ok(0, "unexpected call\n");
-    return E_NOTIMPL;
+    IRecordInfoImpl* This = impl_from_IRecordInfo(iface);
+    This->getsize++;
+    *size = 0;
+    return S_OK;
 }
 
 static HRESULT WINAPI RecordInfo_GetTypeInfo(IRecordInfo *iface, ITypeInfo **ppTypeInfo)
@@ -277,6 +282,10 @@ static IRecordInfoImpl *get_test_recordinfo(void)
     rec = HeapAlloc(GetProcessHeap(), 0, sizeof(IRecordInfoImpl));
     rec->IRecordInfo_iface.lpVtbl = &RecordInfoVtbl;
     rec->ref = 1;
+    rec->recordclear = 0;
+    rec->getsize = 0;
+    rec->recordcopy = 0;
+
     return rec;
 }
 
@@ -765,6 +774,8 @@ static void test_VariantClear(void)
 
 static void test_VariantCopy(void)
 {
+  struct __tagBRECORD *rec;
+  IRecordInfoImpl *recinfo;
   VARIANTARG vSrc, vDst;
   VARTYPE vt;
   size_t i;
@@ -880,6 +891,33 @@ static void test_VariantCopy(void)
     }
     VariantClear(&vDst);
   }
+
+  /* copy RECORD */
+  recinfo = get_test_recordinfo();
+
+  memset(&vDst, 0, sizeof(vDst));
+  V_VT(&vDst) = VT_EMPTY;
+
+  V_VT(&vSrc) = VT_RECORD;
+  rec = &V_UNION(&vSrc, brecVal);
+  rec->pRecInfo = &recinfo->IRecordInfo_iface;
+  rec->pvRecord = (void*)0xdeadbeef;
+
+  recinfo->recordclear = 0;
+  recinfo->recordcopy = 0;
+  recinfo->getsize = 0;
+  recinfo->rec = rec;
+  hres = VariantCopy(&vDst, &vSrc);
+  ok(hres == S_OK, "ret %08x\n", hres);
+
+  rec = &V_UNION(&vDst, brecVal);
+  ok(rec->pvRecord != (void*)0xdeadbeef && rec->pvRecord != NULL, "got %p\n", rec->pvRecord);
+  ok(rec->pRecInfo == &recinfo->IRecordInfo_iface, "got %p\n", rec->pRecInfo);
+  ok(recinfo->getsize == 1, "got %d\n", recinfo->recordclear);
+  ok(recinfo->recordcopy == 1, "got %d\n", recinfo->recordclear);
+
+  VariantClear(&vDst);
+  VariantClear(&vSrc);
 }
 
 /* Determine if a vt is valid for VariantCopyInd() */
diff --git a/dlls/oleaut32/variant.c b/dlls/oleaut32/variant.c
index d4c6f6c..92cf01b 100644
--- a/dlls/oleaut32/variant.c
+++ b/dlls/oleaut32/variant.c
@@ -694,34 +694,32 @@ HRESULT WINAPI VariantClear(VARIANTARG* pVarg)
 /******************************************************************************
  * Copy an IRecordInfo object contained in a variant.
  */
-static HRESULT VARIANT_CopyIRecordInfo(struct __tagBRECORD* pBr)
+static HRESULT VARIANT_CopyIRecordInfo(VARIANT *dest, VARIANT *src)
 {
-  HRESULT hres = S_OK;
+  struct __tagBRECORD *dest_rec = &V_UNION(dest, brecVal);
+  struct __tagBRECORD *src_rec = &V_UNION(src, brecVal);
+  HRESULT hr = S_OK;
+  ULONG size;
 
-  if (pBr->pRecInfo)
+  if (!src_rec->pRecInfo)
   {
-    ULONG ulSize;
+    if (src_rec->pvRecord) return E_INVALIDARG;
+    return S_OK;
+  }
 
-    hres = IRecordInfo_GetSize(pBr->pRecInfo, &ulSize);
-    if (SUCCEEDED(hres))
-    {
-      PVOID pvRecord = HeapAlloc(GetProcessHeap(), 0, ulSize);
-      if (!pvRecord)
-        hres = E_OUTOFMEMORY;
-      else
-      {
-        memcpy(pvRecord, pBr->pvRecord, ulSize);
-        pBr->pvRecord = pvRecord;
+  hr = IRecordInfo_GetSize(src_rec->pRecInfo, &size);
+  if (FAILED(hr)) return hr;
 
-        hres = IRecordInfo_RecordCopy(pBr->pRecInfo, pvRecord, pvRecord);
-        if (SUCCEEDED(hres))
-          IRecordInfo_AddRef(pBr->pRecInfo);
-      }
-    }
-  }
-  else if (pBr->pvRecord)
-    hres = E_INVALIDARG;
-  return hres;
+  /* This could look cleaner if only RecordCreate() was used, but native doesn't use it.
+     Memory should be allocated in a same way as RecordCreate() does, so RecordDestroy()
+     could free it later. */
+  dest_rec->pvRecord = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, size);
+  if (!dest_rec->pvRecord) return E_OUTOFMEMORY;
+
+  dest_rec->pRecInfo = src_rec->pRecInfo;
+  IRecordInfo_AddRef(src_rec->pRecInfo);
+
+  return IRecordInfo_RecordCopy(src_rec->pRecInfo, src_rec->pvRecord, dest_rec->pvRecord);
 }
 
 /******************************************************************************
@@ -771,29 +769,25 @@ HRESULT WINAPI VariantCopy(VARIANTARG* pvargDest, VARIANTARG* pvargSrc)
 
     if (!V_ISBYREF(pvargSrc))
     {
-      if (V_ISARRAY(pvargSrc))
-      {
-        if (V_ARRAY(pvargSrc))
-          hres = SafeArrayCopy(V_ARRAY(pvargSrc), &V_ARRAY(pvargDest));
-      }
-      else if (V_VT(pvargSrc) == VT_BSTR)
+      switch (V_VT(pvargSrc))
       {
+      case VT_BSTR:
         V_BSTR(pvargDest) = SysAllocStringByteLen((char*)V_BSTR(pvargSrc), SysStringByteLen(V_BSTR(pvargSrc)));
         if (!V_BSTR(pvargDest))
-	{
-	  TRACE("!V_BSTR(pvargDest), SysAllocStringByteLen() failed to allocate %d bytes\n", SysStringByteLen(V_BSTR(pvargSrc)));
           hres = E_OUTOFMEMORY;
-	}
-      }
-      else if (V_VT(pvargSrc) == VT_RECORD)
-      {
-        hres = VARIANT_CopyIRecordInfo(&V_UNION(pvargDest,brecVal));
-      }
-      else if (V_VT(pvargSrc) == VT_DISPATCH ||
-               V_VT(pvargSrc) == VT_UNKNOWN)
-      {
+        break;
+      case VT_RECORD:
+        hres = VARIANT_CopyIRecordInfo(pvargDest, pvargSrc);
+        break;
+      case VT_DISPATCH:
+      case VT_UNKNOWN:
+        V_UNKNOWN(pvargDest) = V_UNKNOWN(pvargSrc);
         if (V_UNKNOWN(pvargSrc))
           IUnknown_AddRef(V_UNKNOWN(pvargSrc));
+        break;
+      default:
+        if (V_ISARRAY(pvargSrc))
+          hres = SafeArrayCopy(V_ARRAY(pvargSrc), &V_ARRAY(pvargDest));
       }
     }
   }
@@ -912,8 +906,7 @@ HRESULT WINAPI VariantCopyInd(VARIANT* pvargDest, VARIANTARG* pvargSrc)
   }
   else if (V_VT(pSrc) == (VT_RECORD|VT_BYREF))
   {
-    V_UNION(pvargDest,brecVal) = V_UNION(pvargSrc,brecVal);
-    hres = VARIANT_CopyIRecordInfo(&V_UNION(pvargDest,brecVal));
+    hres = VARIANT_CopyIRecordInfo(pvargDest, pvargSrc);
   }
   else if (V_VT(pSrc) == (VT_DISPATCH|VT_BYREF) ||
            V_VT(pSrc) == (VT_UNKNOWN|VT_BYREF))