- Fix PropVariantClear to reject invalid types.

- Don't crash on NULL pointers.
- Add test case.

dlls/ole32/ole2.c

@@ -2329,23 +2329,90 @@ static void OLE_FreeClipDataArray(ULONG count, CLIPDATA * pClipDataArray)
 
 HRESULT WINAPI FreePropVariantArray(ULONG,PROPVARIANT*);
 
+/******************************************************************************
+ * Check if a PROPVARIANT's type is valid.
+ */
+static inline HRESULT PROPVARIANT_ValidateType(VARTYPE vt)
+{
+    switch (vt)
+    {
+    case VT_EMPTY:
+    case VT_NULL:
+    case VT_I2:
+    case VT_I4:
+    case VT_R4:
+    case VT_R8:
+    case VT_CY:
+    case VT_DATE:
+    case VT_BSTR:
+    case VT_ERROR:
+    case VT_BOOL:
+    case VT_UI1:
+    case VT_UI2:
+    case VT_UI4:
+    case VT_I8:
+    case VT_UI8:
+    case VT_LPSTR:
+    case VT_LPWSTR:
+    case VT_FILETIME:
+    case VT_BLOB:
+    case VT_STREAM:
+    case VT_STORAGE:
+    case VT_STREAMED_OBJECT:
+    case VT_STORED_OBJECT:
+    case VT_BLOB_OBJECT:
+    case VT_CF:
+    case VT_CLSID:
+    case VT_I2|VT_VECTOR:
+    case VT_I4|VT_VECTOR:
+    case VT_R4|VT_VECTOR:
+    case VT_R8|VT_VECTOR:
+    case VT_CY|VT_VECTOR:
+    case VT_DATE|VT_VECTOR:
+    case VT_BSTR|VT_VECTOR:
+    case VT_ERROR|VT_VECTOR:
+    case VT_BOOL|VT_VECTOR:
+    case VT_VARIANT|VT_VECTOR:
+    case VT_UI1|VT_VECTOR:
+    case VT_UI2|VT_VECTOR:
+    case VT_UI4|VT_VECTOR:
+    case VT_I8|VT_VECTOR:
+    case VT_UI8|VT_VECTOR:
+    case VT_LPSTR|VT_VECTOR:
+    case VT_LPWSTR|VT_VECTOR:
+    case VT_FILETIME|VT_VECTOR:
+    case VT_CF|VT_VECTOR:
+    case VT_CLSID|VT_VECTOR:
+        return S_OK;
+    }
+    WARN("Bad type %d\n", vt);
+    return STG_E_INVALIDPARAMETER;
+}
+
 /***********************************************************************
  *           PropVariantClear			    [OLE32.@]
  */
 HRESULT WINAPI PropVariantClear(PROPVARIANT * pvar) /* [in/out] */
 {
+    HRESULT hr;
+
     TRACE("(%p)\n", pvar);
 
     if (!pvar)
         return S_OK;
 
+    hr = PROPVARIANT_ValidateType(pvar->vt);
+    if (FAILED(hr))
+        return hr;
+
     switch(pvar->vt)
     {
     case VT_STREAM:
     case VT_STREAMED_OBJECT:
     case VT_STORAGE:
     case VT_STORED_OBJECT:
-        IUnknown_Release((LPUNKNOWN)pvar->u.pStream);
+        if (pvar->u.pStream)
+            IUnknown_Release(pvar->u.pStream);
         break;
     case VT_CLSID:
     case VT_LPSTR:
@@ -2370,12 +2437,7 @@ HRESULT WINAPI PropVariantClear(PROPVARIANT * pvar) /* [in/out] */
         }
         break;
     default:
-        if (pvar->vt & VT_ARRAY)
+        switch (pvar->vt & ~VT_VECTOR)
-        {
-            FIXME("Need to call SafeArrayDestroy\n");
-            /* SafeArrayDestroy(pvar->u.caub); */
-        }
-        switch (pvar->vt & VT_VECTOR)
         {
         case VT_VARIANT:
             FreePropVariantArray(pvar->u.capropvar.cElems, pvar->u.capropvar.pElems);
@@ -2386,9 +2448,10 @@ HRESULT WINAPI PropVariantClear(PROPVARIANT * pvar) /* [in/out] */
         case VT_BSTR:
         case VT_LPSTR:
         case VT_LPWSTR:
+        case VT_CLSID:
             FIXME("Freeing of vector sub-type not supported yet\n");
         }
-        if (pvar->vt & VT_VECTOR)
+        if (pvar->vt & ~VT_VECTOR)
         {
             /* pick an arbitary VT_VECTOR structure - they all have the same
              * memory layout */
@@ -2408,7 +2471,13 @@ HRESULT WINAPI PropVariantCopy(PROPVARIANT *pvarDest,      /* [out] */
                                const PROPVARIANT *pvarSrc) /* [in] */
 {
     ULONG len;
-    TRACE("(%p, %p): stub:\n", pvarDest, pvarSrc);
+    HRESULT hr;
+
+    TRACE("(%p, %p)\n", pvarDest, pvarSrc);
+
+    hr = PROPVARIANT_ValidateType(pvarSrc->vt);
+    if (FAILED(hr))
+        return hr;
 
     /* this will deal with most cases */
     CopyMemory(pvarDest, pvarSrc, sizeof(*pvarDest));
@@ -2456,11 +2525,6 @@ HRESULT WINAPI PropVariantCopy(PROPVARIANT *pvarDest,      /* [out] */
         }
         break;
     default:
-        if (pvarSrc->vt & VT_ARRAY)
-        {
-            FIXME("Need to call SafeArrayCopy\n");
-            /* SafeArrayCopy(...); */
-        }
         if (pvarSrc->vt & VT_VECTOR)
         {
             int elemSize;

dlls/ole32/tests/.cvsignore

@@ -1,3 +1,4 @@
 Makefile
+propvariant.ok
 storage32.ok
 testlist.c

dlls/ole32/tests/Makefile.in

@@ -7,6 +7,7 @@ IMPORTS   = ole32 kernel32 ntdll
 EXTRALIBS = -luuid
 
 CTESTS = \
+	propvariant.c \
 	storage32.c
 
 @MAKE_TEST_RULES@

dlls/ole32/tests/propvariant.c

@@ -0,0 +1,166 @@
+/*
+ * PropVariant Tests
+ *
+ * Copyright 2004 Robert Shearman
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include "windows.h"
+/* not present in Wine yet */
+/*#include "propidl.h"*/
+WINOLEAPI PropVariantClear(PROPVARIANT*);
+
+#include "wine/test.h"
+
+struct valid_mapping
+{
+    BOOL simple;
+    BOOL with_array;
+    BOOL with_vector;
+    BOOL byref;
+} valid_types[] =
+{
+    { TRUE , FALSE, FALSE, FALSE }, /* VT_EMPTY */
+    { TRUE , FALSE, FALSE, FALSE }, /* VT_NULL */
+    { TRUE , FALSE, TRUE , FALSE }, /* VT_I2 */
+    { TRUE , FALSE, TRUE , FALSE }, /* VT_I4 */
+    { TRUE , FALSE, TRUE , FALSE }, /* VT_R4 */
+    { TRUE , FALSE, TRUE , FALSE }, /* VT_R8 */
+    { TRUE , FALSE, TRUE , FALSE }, /* VT_CY */
+    { TRUE , FALSE, TRUE , FALSE }, /* VT_DATE */
+    { TRUE , FALSE, TRUE , FALSE }, /* VT_BSTR */
+    { FALSE, FALSE, FALSE, FALSE }, /* VT_DISPATCH */
+    { TRUE , FALSE, TRUE , FALSE }, /* VT_ERROR */
+    { TRUE , FALSE, TRUE , FALSE }, /* VT_BOOL */
+    { FALSE, FALSE, TRUE , FALSE }, /* VT_VARIANT */
+    { FALSE, FALSE, FALSE, FALSE }, /* VT_UNKNOWN */
+    { FALSE, FALSE, FALSE, FALSE }, /* VT_DECIMAL */
+    { FALSE, FALSE, FALSE, FALSE }, /* 15 */
+    { FALSE, FALSE, FALSE, FALSE }, /* VT_I1 */
+    { TRUE , FALSE, TRUE , FALSE }, /* VT_UI1 */
+    { TRUE , FALSE, TRUE , FALSE }, /* VT_UI2 */
+    { TRUE , FALSE, TRUE , FALSE }, /* VT_UI4 */
+    { TRUE , FALSE, TRUE , FALSE }, /* VT_I8 */
+    { TRUE , FALSE, TRUE , FALSE }, /* VT_UI8 */
+    { FALSE, FALSE, FALSE, FALSE }, /* VT_INT */
+    { FALSE, FALSE, FALSE, FALSE }, /* VT_UINT */
+    { FALSE, FALSE, FALSE, FALSE }, /* VT_VOID */
+    { FALSE, FALSE, FALSE, FALSE }, /* VT_HRESULT */
+    { FALSE, FALSE, FALSE, FALSE }, /* VT_PTR */
+    { FALSE, FALSE, FALSE, FALSE }, /* VT_SAFEARRAY */
+    { FALSE, FALSE, FALSE, FALSE }, /* VT_CARRAY */
+    { FALSE, FALSE, FALSE, FALSE }, /* VT_USERDEFINED */
+    { TRUE , FALSE, TRUE , FALSE }, /* VT_LPSTR */
+    { TRUE , FALSE, TRUE , FALSE }, /* VT_LPWSTR */
+    { FALSE, FALSE, FALSE, FALSE }, /* 32 */
+    { FALSE, FALSE, FALSE, FALSE }, /* 33 */
+    { FALSE, FALSE, FALSE, FALSE }, /* 34 */
+    { FALSE, FALSE, FALSE, FALSE }, /* 35 */
+    { FALSE, FALSE, FALSE, FALSE }, /* VT_RECORD */
+    { FALSE, FALSE, FALSE, FALSE }, /* VT_INT_PTR */
+    { FALSE, FALSE, FALSE, FALSE }, /* VT_UINT_PTR */
+    { FALSE, FALSE, FALSE, FALSE }, /* 39 */
+    { FALSE, FALSE, FALSE, FALSE }, /* 40 */
+    { FALSE, FALSE, FALSE, FALSE }, /* 41 */
+    { FALSE, FALSE, FALSE, FALSE }, /* 42 */
+    { FALSE, FALSE, FALSE, FALSE }, /* 43 */
+    { FALSE, FALSE, FALSE, FALSE }, /* 44 */
+    { FALSE, FALSE, FALSE, FALSE }, /* 45 */
+    { FALSE, FALSE, FALSE, FALSE }, /* 46 */
+    { FALSE, FALSE, FALSE, FALSE }, /* 47 */
+    { FALSE, FALSE, FALSE, FALSE }, /* 48 */
+    { FALSE, FALSE, FALSE, FALSE }, /* 49 */
+    { FALSE, FALSE, FALSE, FALSE }, /* 50 */
+    { FALSE, FALSE, FALSE, FALSE }, /* 51 */
+    { FALSE, FALSE, FALSE, FALSE }, /* 52 */
+    { FALSE, FALSE, FALSE, FALSE }, /* 53 */
+    { FALSE, FALSE, FALSE, FALSE }, /* 54 */
+    { FALSE, FALSE, FALSE, FALSE }, /* 55 */
+    { FALSE, FALSE, FALSE, FALSE }, /* 56 */
+    { FALSE, FALSE, FALSE, FALSE }, /* 57 */
+    { FALSE, FALSE, FALSE, FALSE }, /* 58 */
+    { FALSE, FALSE, FALSE, FALSE }, /* 59 */
+    { FALSE, FALSE, FALSE, FALSE }, /* 60 */
+    { FALSE, FALSE, FALSE, FALSE }, /* 61 */
+    { FALSE, FALSE, FALSE, FALSE }, /* 62 */
+    { FALSE, FALSE, FALSE, FALSE }, /* 63 */
+    { TRUE , FALSE, TRUE , FALSE }, /* VT_FILETIME */
+    { TRUE , FALSE, FALSE, FALSE }, /* VT_BLOB */
+    { TRUE , FALSE, FALSE, FALSE }, /* VT_STREAM */
+    { TRUE , FALSE, FALSE, FALSE }, /* VT_STORAGE */
+    { TRUE , FALSE, FALSE, FALSE }, /* VT_STREAMED_OBJECT */
+    { TRUE , FALSE, FALSE, FALSE }, /* VT_STORED_OBJECT */
+    { TRUE , FALSE, FALSE, FALSE }, /* VT_BLOB_OBJECT */
+    { TRUE , FALSE, TRUE , FALSE }  /* VT_CF */
+};
+
+static const char* wine_vtypes[VT_CLSID+1] =
+{
+  "VT_EMPTY","VT_NULL","VT_I2","VT_I4","VT_R4","VT_R8","VT_CY","VT_DATE",
+  "VT_BSTR","VT_DISPATCH","VT_ERROR","VT_BOOL","VT_VARIANT","VT_UNKNOWN",
+  "VT_DECIMAL","15","VT_I1","VT_UI1","VT_UI2","VT_UI4","VT_I8","VT_UI8",
+  "VT_INT","VT_UINT","VT_VOID","VT_HRESULT","VT_PTR","VT_SAFEARRAY",
+  "VT_CARRAY","VT_USERDEFINED","VT_LPSTR","VT_LPWSTR","32","33","34","35",
+  "VT_RECORD","VT_INT_PTR","VT_UINT_PTR","39","40","41","42","43","44","45",
+  "46","47","48","49","50","51","52","53","54","55","56","57","58","59","60",
+  "61","62","63","VT_FILETIME","VT_BLOB","VT_STREAM","VT_STORAGE",
+  "VT_STREAMED_OBJECT","VT_STORED_OBJECT","VT_BLOB_OBJECT","VT_CF","VT_CLSID"
+};
+
+static void test_validtypes()
+{
+    PROPVARIANT propvar;
+    HRESULT hr;
+    int i;
+
+    memset(&propvar, 0, sizeof(propvar));
+
+    for (i = 0; i < sizeof(valid_types)/sizeof(valid_types[0]); i++)
+    {
+        propvar.vt = i;
+        hr = PropVariantClear(&propvar);
+        ok(valid_types[i].simple == !(hr == STG_E_INVALIDPARAMETER),
+            "PropVariantClear(%s) should have returned 0x%08lx, but returned 0x%08lx\n",
+            wine_vtypes[i],
+            valid_types[i].simple ? S_OK : STG_E_INVALIDPARAMETER, hr);
+
+        propvar.vt = i | VT_ARRAY;
+        hr = PropVariantClear(&propvar);
+        ok(valid_types[i].with_array == !(hr == STG_E_INVALIDPARAMETER),
+            "PropVariantClear(%s|VT_ARRAY) should have returned 0x%08lx, but returned 0x%08lx\n",
+            wine_vtypes[i],
+            valid_types[i].with_array ? S_OK : STG_E_INVALIDPARAMETER, hr);
+
+        propvar.vt = i | VT_VECTOR;
+        hr = PropVariantClear(&propvar);
+        ok(valid_types[i].with_vector == !(hr == STG_E_INVALIDPARAMETER),
+            "PropVariantClear(%s|VT_VECTOR) should have returned 0x%08lx, but returned 0x%08lx\n",
+            wine_vtypes[i],
+            valid_types[i].with_vector ? S_OK : STG_E_INVALIDPARAMETER, hr);
+
+        propvar.vt = i | VT_BYREF;
+        hr = PropVariantClear(&propvar);
+        ok(valid_types[i].byref == !(hr == STG_E_INVALIDPARAMETER),
+            "PropVariantClear(%s|VT_BYREF) should have returned 0x%08lx, but returned 0x%08lx\n",
+            wine_vtypes[i],
+            valid_types[i].byref ? S_OK : STG_E_INVALIDPARAMETER, hr);
+    }
+}
+
+START_TEST(propvariant)
+{
+    test_validtypes();
+}