[v3 03/12] comctl32: Added basic implementation for task dialogs and add tests
Nikolay Sivov
bunglehead at gmail.com
Fri Mar 10 15:57:14 CST 2017
On 10.03.2017 21:21, Fabian Maurer wrote:
> + UINT text_size; /* Length in bytes including null-terminator */
> + UINT class_size; /* Length in bytes including null-terminator */
> + struct list entry;
> +}control_info;
> +
> +typedef struct
> +{
> + DLGTEMPLATE template;
> + WORD menu;
> + WORD class;
> + const WCHAR *title;
> +
> + /* Not part of actual template */
> + UINT titleSize; /* Length in bytes including null-terminator */
> +}dialog_header;
About formatting issues, for consistency please add space after bracket,
same goes for 'if' and other places.
"titleSize" -> "title_size".
> +#define MEMCPY_MOVEPTR(target, source, size) memcpy(target, source, size); target += size;
I think this one will be better as a function, also this macro assumes
that target point is char*, which is impossible to guess.
> +static void* align_pointer(void *ptr, unsigned int boundary)
> +{
> + boundary--;
> + return (void *)(((UINT_PTR)ptr + boundary) & ~boundary);
> +}
This is usually done with macros in Wine I think, like
((x) + size - 1) & ~(size - 1).
> +LPDLGTEMPLATEW dialog_template_create(dialog_header header, struct list *controls)
Why is 'header' passed by value?
> + void *template_data_start;
I think it's better to declare this with a type you're going to return.
> + data_size += sizeof(WORD)*2 + header.titleSize;
Same formatting issue - please add spaces around operator.
> + /* 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?
More information about the wine-devel
mailing list