Re: [PATCH v2 03/15] propsys: Extend PropVariantCompareEx()

[Nikolay Sivov]

On 18.07.2015 19:26, Jonas Kümmerlin wrote:
> - case VT_CLSID:
> -
>    /* IsEqualUUID only compares for equality:(  */
> -
>    CMP_INT_VALUE(puuid->Data1);
> -
>    if (res)
> -
>        break;
> -
> -
>    CMP_INT_VALUE(puuid->Data2);
> -
>    if (res)
> -
>        break;
> -
> -
>    CMP_INT_VALUE(puuid->Data3);
> -
>    if (res)
> -
>        break;
> -
> -
>    res = memcmp(propvar1->u.puuid->Data4, propvar2_converted->u.puuid->Data4, 8);
> -
>    break;

You sure you can't just memcmp() whole structure?
> - case VT_BSTR:   /* BSTR and LPWSTR are NOT EQUAL in general, but here */
> - case VT_LPWSTR:
>     /* FIXME: Use string flags. */
> -
>    res = lstrcmpW(propvar1->u.bstrVal, propvar2->u.bstrVal);
> -
>    res = lstrcmpW(propvar1->u.bstrVal, propvar2_converted->u.bstrVal);
> -
>    break;
> - case VT_VECTOR|VT_BSTR:
> - case VT_VECTOR|VT_LPWSTR:
> - {
> -
>    ULONG i;
> -
> -
>    CMP_INT_VALUE(calpwstr.cElems);
> -
>    if (res)
> -
>        break;
> -
> -
>    for (i = 0; i < propvar1->u.calpwstr.cElems; ++i)
> -
>    {
> -
>        /* FIXME: Use string flags. */
> -
>        res = lstrcmpW(propvar1->u.calpwstr.pElems[i], propvar2_converted->u.calpwstr.pElems[i]);
> -
> -
>        if (res)
> -
>            break;
> -
>    }
> -
>    break;
> - }

I really think common vector bits should be done in one place (after 
tests show they all behave same way of course), as vector structure is 
always the same - cElems is always a first field. Regarding strings, 
it's better to use flags and call proper functions from the start.

P.S. it's up to you but maybe you could submit some obviously correct 
parts like Init* and header changes to wine-patches so we can reduce the 
diff?