[v3 03/12] comctl32: Added basic implementation for task dialogs and add tests

Nikolay Sivov bunglehead at gmail.com
Sun Mar 12 13:35:47 CDT 2017


On 11.03.2017 1:26, Fabian Maurer wrote:
>> This is usually done with macros in Wine I think, like
>>
>> ((x) + size - 1) & ~(size - 1).
> Do we already have a macro like that I should use or should I just define it 
> like that?

Not in shared headers, some dlls have their own, for example
ole32/usrmarshal.c does.

> 
>>> +LPDLGTEMPLATEW dialog_template_create(dialog_header header, struct list
>>> *controls)
>> Why is 'header' passed by value?
> Because it doesn't really matter. Is there a benefit passing it as pointer?

Looks odd to me, that's all. I think this could actually be improved by
merging controls list with 'header' fields, and calling it dialog
description or something like that.

> 
>>> +    /* Align on WORD boundary for the strings */
>>> +    template_data = align_pointer(template_data, 2);
>>
>> I don't see how it could be not aligned to WORD, could you explain?
> I wasn't sure about the alignment of LocalAlloc, so I added that check. Does 
> it guarantee an aligned pointer?

GlobalAlloc() is documented to align at 8 bytes, and in practice
HeapAlloc() does the same I think.

> 
> Regarding your points I didn't comment on, I'll fix those in the next version.
> 
> Regards
> Fabian Maurer
> 
> 




More information about the wine-devel mailing list