[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