On Wed, Jul 9, 2008 at 11:21 AM, Michael Karcher <wine(a)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. -- James Hawkins