On Wed, Jul 9, 2008 at 11:21 AM, Michael Karcher wine@mkarcher.dialup.fu-berlin.de wrote:
Hello Ismael,
reviewing this patch shows some issues.
> +#define _okHR(expected, result) \
- ok( (expected) == (result), \
"expected=%d(%s) got=%d(%s)\n", \
HRESULT_CODE(expected), dpResult2str(expected), \
HRESULT_CODE(result), dpResult2str(result) );
I dislike the order of the parameters and the name. You would write ok( hr == DP_OK, "Something failed with %x\n", hr); In this line, "hr" is before "DP_OK". So as an analogy, I also want to write _okHR( hr, DD_OK );
Why do you start the macro name with an underscore? And last but not least: As I see it, a static inline would do the job too, and is always preferrable to macros.
We use macros for this (ok*/expected) all over the code base whenever the check is small enough. See dlls/comctl32/tests/trackbar.c:28 for an example. If you use an inlined function, you either have to pass the __LINE__ value to the function or wrap the call to the function in a macro anyway.