On Wed, 13 Jan 2016, Hugh McMaster wrote:
On Tue, 12 Jan 2016 12:58:59 +0100, Francois Gouget wrote:
On Tue, 12 Jan 2016, Hugh McMaster wrote: [...]
diff --git a/programs/regsvr32/tests/Makefile.in b/programs/regsvr32/tests/Makefile.in new file mode 100644 index 0000000..61d25fe --- /dev/null +++ b/programs/regsvr32/tests/Makefile.in
[...]
+ifneq (,$(filter crosstest mingw,$(MAKECMDGOALS) $(findstring mingw,$(CC))))
- TARGET_DLLS = $(addsuffix .dll, $(DLL_BASENAMES))
I believe the syntax on these last two lines is specific to GNU make which is something that Wine has tried to avoid so far.
The GNU make manual doesn't list 'filter', 'findstring' or 'addsuffix' in its comparison with BSD's (p)make, so I'm not sure about those functions being specific or not.
I believe a more common way to do what addsuffix does is:
TARGET_DLLS = $(DLL_BASENAMES:%=%.dll)
But there is also ifneq, it seems the syntax is different between GNU make and BSD make: https://stackoverflow.com/questions/9096018/make-gmake-compatible-if-else-st...
[...]
My preference would be to only use mingw32, but I don't think the mingw32 package is a build-dependency for Wine. How likely is it that users building from source will have mingw32-w64 (or equivalent) installed?
MinGW is not used to compile Wine or the conformance tests. It is only used for cross-compiling the tests for Windows so very few Wine developpers will have it. More importantly Wine should not depend on it.
[...]
The existing code embeds the test DLLs in a resource file. The difficulty is whether to build two resource files (one for Wine, the other for Windows), or add the resource to .PHONY and have it rebuilt for each target.
dlls/kernel32/tests uses the same .res file for both cases but it probably does not apply to your case.
Your case is probably more similar to programs/winetest and there is no cross-building there. Instead to build the Windows executable one has to have a separate tree and cross-build all of it (iirc).
[...]
+#define run_test(...) run_test_(output, &r, __VA_ARGS__)
A macro with a variable argument list? That does not seem portable (or maybe is a> C99 feature).
It is a C99 feature. We already use it in comctl32/tests, xaudio2_7/tests, the Mac driver and wine/debug.h.
I'd say the one trustworthy reference of those is wine/debug.h. Ok, so that's probably acceptable.
However, as far as I can tell its only purpose is to hide which variables are being modified by run_test_(). Furthermore it only works if the function using it has defined local variables with the right names. So all it does is obfuscate the source code making it a net negative. So it should go.
[...]
+#define CMP(s) !strcmp(output, s)
This macro is totally unnecessary.
Yes, you're right. I'll define a new macro re-using the string in the ok() test.
Macros have big drawbacks: they obscure what is really happening, have no type checking, can have side-effects, may execute expressions in their parameters more than once. They should only be used when really needed. The case at hand can simply be done with no macro at all:
ok(strcmp(s, "Register") == 0, "got '%s', expected 'Register'\n", output);
Now if there was really a lot of these string checks, one could argue for a macro of the form:
#define okString(s, e) ok(strcmp((s), (e)) == 0, "got '%s', expected '%s'\n", (s), (e))
Then you'd write: okString(s, "Register");
At least you could then argue that the resulting code really is shorter and that it does not hide which variable is compared to the expected value.