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