[PATCH 15/15] api-ms-win-core-winrt-string: Implement a few HSTRING functions.

Sebastian Lackner sebastian at fds-team.de
Mon Dec 1 05:06:48 CST 2014


On 01.12.2014 11:27, Martin Storsjo wrote:
> +
> +    buf = malloc(sizeof(*buf));
> +    if (!buf)
> +        return E_OUTOFMEMORY;
> +    buf->buffer = malloc((length + 1) * sizeof(*buf->buffer));
> +    if (!buf->buffer) {
> +        free(buf);
> +        return E_OUTOFMEMORY;
> +    }

Some of your previous patches also contain changes to ./configure which is an autogenerated file. Afaik its not allowed to include that when submitting patches.

Feedback for this part:

* HSTRING and HSTRING_HEADER should probably be defined in some include fine. To ensure that the size of private structure matches you could also use C_ASSERT(...), and maybe even disable align of the structure fields.

* You should avoid using malloc(...) in Wine source

* http://msdn.microsoft.com/en-us/library/br224632(v=vs.85).aspx mentions:
"""Calling WindowsDeleteString decrements the reference count of the backing buffer, and if the reference count reaches 0, the Windows Runtime de-allocates the buffer."""

I don't see the concept of refcounted backing buffers anywhere in your code, which is most likely wrong. Its difficult to fix that later.

* I would suggest starting with some stub functions, and then add tests before sending the actual implementation.

BTW: Michael Müller was also working on some api-ms-win-core-* stubs, but I think he is also fine when your patches get upstream instead:
https://github.com/wine-compholio/wine-staging/tree/master/patches/api-ms-win-core-Stubs

Regards,
Sebastian



More information about the wine-devel mailing list