Re: [5/6] windowscodecs: Implement IPropertyBag2::Write

[Vincent Povirk]

if (This->properties[i].pstrName)
             {
                 HeapFree(GetProcessHeap(), 0, This->properties[i].pstrName);
+                VariantClear( This->values+i ); /* Not initialized if
pstrName not initialized */
             }

...

{
         This->properties = HeapAlloc(GetProcessHeap(),
HEAP_ZERO_MEMORY, sizeof(PROPBAG2)*count);
-
-        if (!This->properties)
+        This->values = HeapAlloc(GetProcessHeap(), 0, sizeof(VARIANT)*count);
+
+        if (!This->properties || !This->values)
             res = E_OUTOFMEMORY;
         else
             for (i=0; i < count; i++)

Well, that explains why you had the NULL check in the previous patch.
If you allocate values with HEAP_ZERO_MEMORY, you don't have to use
VariantInit, and you could simplify your Release code (at least in
terms of the effort required to verify that it is correct). I think
the way it is now is technically correct though.

+        {
+            if (pPropBag[i].pstrName)
+                FIXME("Application tried to set the unknown option %s.\n",
+                      debugstr_w(pPropBag[i].pstrName));
+                /* FIXME: Function is not atomar on error, but MSDN
does not say anything about it
+                 *        (no reset of items between 0 and i-1) */
+                return E_FAIL;
+        }

Bit of an indentation glitch there - the return statement isn't (and
shouldn't be) in the if statement.