You'll probably have to split this into smaller patches.

+            return E_FAIL; /* FIXME: Function is not atomar on error,
but MSDN does not say anything about it */

I don't understand. Are you saying native leaks memory on error, and
if so how do you know this?

+                FIXME("Application tried to set the unknown option %s. "
+                "This may be an application error but is probably
caused by an incomplete implementation in wine\n",
+                debugstr_w(pPropBag[i].pstrName));

I don't think the extra explanation of why this is a FIXME adds anything.

+            if (This->properties[idx].vt != V_VT(pvarValue+i))
+                return E_FAIL;

Are you sure this shouldn't be converted?

+            while (TRUE)
+            {
+                CoTaskMemFree( pPropBag[--i].pstrName );
+                if(i == 0)
+                    break;
+            }

This could be written more simply as a do/while loop.

+extern HRESULT CreateEncoderPropertyBag(PROPBAG2 *options, UINT
count, IPropertyBag2 **property) DECLSPEC_HIDDEN;

This adds code that's not really used. You might want to start out by
changing the arguments of CreatePropertyBag2 and adding the
IWICComponentFactory method. Then you can write tests first and remove
todo's as you go. I don't think there's any reason to keep a
constructor with the old set of arguments around.


Reply via email to