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.