[PATCH 3/6] ole32, propsys: Rework PROPVARIANT (de)serialization

[Nikolay Sivov]

On 16.07.2015 11:09, Jonas Kümmerlin wrote:
> Nikolay Sivov wrote:
>> On 14.07.2015 22:47, Jonas Kümmerlin wrote:
>>> +@ stdcall wine_WritePropertyToBuffer(ptr ptr long ptr long long)
>>> +@ stdcall wine_SerializedPropertySize(ptr ptr long long)
>>> +@ stdcall wine_ReadProperty(ptr ptr long ptr long long ptr ptr)
>>
>> We avoid custom exports, unless there's absolutely no way to go
>> without
>> them. If you're sure public API is not enough, you should consider
>> duplication of relevant parts.
>
> The problem at hand:
>        * propsys.dll has to do property serialization/deserialization,
>          at least for getting Stg(De)SerializePropVariant() to work.

Sure.

>        * Implementing it twice is not really appealing since it amounts
>          to >1000 lines of code.

I mentioned it as an option to consider in case no public API exists to 
do same job.

>        * The only interface to property (de)serialization provided by
>          ole32.dll, the StgConvertVariantToProperty()/
>          StgConvertPropertyToVariant() functions, are poorly documented,
>          hard to implement (HRESULTs need to be converted to NTSTATUS
>          exceptions) and even more unpleasant to use (because one would
>          need to catch the NTSTATUS exception and convert it back to a
>          HRESULT). This is rather crazy and cumbersome.

I wouldn't say they are that poorly documented, all used types and 
arguments are documented, prototypes are known. If they actually do what 
you need it sounds like a best option. The fact that they raise an 
exception instead of returning and error code is not that uncommon, Ndr* 
marshalling code does the same, and it actually simplifies error 
handling for client if serialization implies calling them sequentially - 
you only need one exception handling block around all of them, once one 
fails you can't/shouldn't try to recover, that's the reason of such 
design most likely. Note though that it's irrelevant if they are 
actually used in native propsys - it's implementation detail, and 
shouldn't rely on it or explore if it's actually the case or not.

First thing to do would be to write some tests for ole32 and propsys 
serialization, tests should of course be separated and go to respective 
tests dir.

>
> How about moving Stg(De)SerializePropVariant() into OLE32.dll and
> reexporting it from propsys.dll? That could solve the problem without
> introducing a completely new API.
>

That's not better than adding custom named exports as you did in your 
patch. The point is to have clean inter-module interface and proper 
separation.