[PATCH 03/18] comctl32: Added basic implementation for task dialogs and add tests

[Nikolay Sivov]

On 26.02.2017 21:39, Fabian Maurer wrote:
> Thanks for the explanation,
> 
>>>> You can't free list node like that.
>>>
>>> What do you mean? I allocated the list and it's contents with HeapAlloc,
>>> so I need to free it that way. Or am I missing something?
>>
>> You need to unlink it first.
> Do I really need to unlink it when I'm freeing everything? Calling list_remove 
> before freeing all the entries doesn't really help us, does it? We don't 
> access the list afterwards anymore.

Yes, you need to unlink. And yes, you do access it, because you keep
iterating through the list after you freed first entry. Take a look how
it's done, this is used a lot all around.

> 
>>>> Static string for that doesn't look right.
>>>
>>> Why is that? I've looked into the other code, and strings are often
>>> defined as static const WCHAR, that was the recommended method, too.
>>> Of course that needs to be changed later, since we (probably) want to take
>>> localization into account.
>>
>> Yes, that's what I mean, it should go to resources.
> Yes, I have that in mind for the future, but can I leave it that way for the 
> first implementation?

It's a matter of a single LoadStringW call, once you added a string as a
resource. Make sure it matches how user32 defines it - as "OK", this way
po files will not require translation updates.

> 
> Regards,
> Fabian Maurer
> 
> 
> 
>