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

[Martin Storsjö]

On Sun, 7 Dec 2014, Sebastian Lackner wrote:

> * Include files should also be added as a separate patch. At the moment 
> the formatting looks a bit ugly (random number of linebreaks?).

I had some sort of grouping of the functions into groups with similar 
functions, but I agree that it looks mostly random.

> * In the specfile use 'wstr' for arguments which are widechar strings. 
> This allows to print them with the +relay debug channel.

Ok - can this be used for strings that aren't null terminated as well 
(like the parameter to WindowsCreateString)?

> * Not sure how strict Wine is about the coding style, but generally a 
> slighly different style is preferred for new code.

Ok, will change

> For patch 4/4:
>
> * C++-Style comments like "// ..." are not allowed in Wine source, 
> instead use "/* ... */" for single-line comments or

Ok, will change

> * To avoid crashes on machines where this library is not available, it is recommended to skip the tests in such a case. For example:
>
> --- snip ---
> hmod = LoadLibraryA(...);
> if (!hmod)
> {
>    win_skip("Failed to load api-ms-win-core-winrt-string-l1-1-0, skipping tests\n");
>    return;
> }
> --- snip ---

Ok, will change

> * It is okay to use define macros for loading function pointers, but 
> they should be defined directly before, and undefined immediately 
> afterwards, to avoid collision with other definitions. I also don't 
> think that checking for != NULL is really necessary, these api-* dlls 
> have a well-defined set of export functions, so it is basically 
> guaranteed that all functions are available.

Yeah, although the checking here doesn't cost much extra effort either, as 
long as it's wrapped into a macro

> * check_string should also show the line number because it is used very 
> often.

Indeed, that will be helpful.

> * I would also suggest to split the tests. There is no need to put everything in a single function, especially because api-*-string only implements string functions. ;)
> I would prefer something like:

Ok, I'll see if I manage to get it split in a sensible way.

// Martin