Re: [PATCH v2 05/15] propsys: Implement Stg(De)SerializePropVariant

[Nikolay Sivov]

On 18.07.2015 19:26, Jonas Kümmerlin wrote:
> - __TRY
> - {
> -
>    StgConvertVariantToProperty(pVar, CP_UNICODE, NULL, &space, 0, FALSE, 0);
> - }
> - __EXCEPT_ALL
> - {
> -
>    WARN("Failed to serialize property of type %d: hr=%08x\n", pVar->vt, hr);
> -
>    return HRESULT_FROM_WIN32(RtlNtStatusToDosError(GetExceptionCode()));
> - }
> - __ENDTRY;
> -
> - *pcb = 0;
> -
> - /* MSDN requires ppProp to be defined, but, as an extension, we allow
> -
> * a NULL value and return the required size instead.
> -
> */
> - if (ppProp) *ppProp = NULL;
> -
> - if (ppProp)
> - {
> -
>    buffer = CoTaskMemAlloc(space);

Why would you need to get only returned size from this? My understanding 
is that you pass it variant, and it returns buffer allocated with 
CoTaskMemAlloc() and size of it. And it looks like it should be enough 
to have single TRY block.
> -
>    __TRY
> -
>    {
> -
>        *ppProp = StgConvertVariantToProperty(pVar, CP_UNICODE, buffer, &space, 0, FALSE, 0);
> -
>        if (!*ppProp)
> -
>            hr = E_FAIL;
> -
>    }
> -
>    __EXCEPT_ALL
> -
>    {
> -
>        WARN("Failed to serialize property of type %d: hr=%08x\n", pVar->vt, hr);
> -
>        *ppProp = NULL;
> -
>        CoTaskMemFree(buffer);
> -
>        return HRESULT_FROM_WIN32(RtlNtStatusToDosError(GetExceptionCode()));
> -
>    }
> -
>    __ENDTRY;
> -
> -
>    if (FAILED(hr))
> -
>    {
> -
>        WARN("Failed to serialize property of type %d: hr=%08x\n", pVar->vt, hr);
> -
>        CoTaskMemFree(buffer);
> -
> -
>        return hr;
> -
>    }

It's probably safe to assume that last FAILED() is unreachable, because 
on any error exception will be raised.
> - else
> - {
> -
>    /* This is a wine extension */
> -
>    *pcb = space;
> - }

It's better to avoid adding such workarounds.