[PATCH] crypt32: Store CRYPT_KEY_PROV_INFO in a platform independent way.

[Alexandre Julliard]

Dmitry Timoshkov <dmitry at baikal.ru> writes:

> Alexandre Julliard <julliard at winehq.org> wrote:
>
>> >> >> >> This makes reading and writing of CRYPT_KEY_PROV_INFO certificate property
>> >> >> >> from/to the certificate storage independent from architecture the crypt32.dll
>> >> >> >> code being executed on.
>> >> >> >
>> >> >> > Is there anything that could be improved to make this patch accepted?
>> >> >> 
>> >> >> That data structure looks strange. Maybe you could explain why it needs
>> >> >> to use the 64-bit layout, and why you are achieving that with padding
>> >> >> instead of using 64-bit types?
>> >> >
>> >> > 64-bit layout is used so that 64-bit crypt32 is able to read the structure
>> >> > saved by 32-bit crypt32, currently this leads to a memory corruption. There's
>> >> > no reason to not use 64-bit types, thanks.
>> >> 
>> >> That's better, but as long as you are defining your own structure, I
>> >> don't see why using simply DWORDs wouldn't work just as well, as long as
>> >> you use the same structure on 32- and 64-bit.
>> >
>> > Obviously DWORDs won't work for pointers, using ULONG64 allows to avoid
>> > reallocation when reading CRYPT_KEY_PROV_INFO from the certificate storage,
>> > and just fix up the pointers in place as current code is doing.
>> 
>> I feel it would be cleaner to do that with some sort of union, and use
>> the correct types and names for what is being stored. In particular
>> keeping names like 'pwszContainerName' for things that are not pointers
>> isn't very meaningful. But of course the existing code is already not
>> great in that respect...
>
> My main intention was to reuse current code, even it's so much ugly,
> that's why I decided to keep using such a structure, and just adapt
> it for mixed 32/64-bit case. If you feel strongly about rewriting it,
> then current code should be completely replaced.

That would be a good idea, if you feel like doing it. But I'll commit
your patch in the meantime.

-- 
Alexandre Julliard
julliard at winehq.org