[1/6] windowscodecs: Implementation of IPropertyBag2

Vincent Povirk madewokherd at gmail.com
Sat Jan 19 13:46:32 CST 2013

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.

More information about the wine-devel mailing list