On Mon, Feb 7, 2022 at 12:27 PM Ricardo Ribalda <ribalda@xxxxxxxxxxxx> wrote:
>
> Today, when we want to check if a pointer is NULL and not ERR we have
> two options:
>
> EXPECT_TRUE(test, ptr == NULL);
>
> or
>
> EXPECT_PTR_NE(test, ptr, (struct mystruct *)NULL);
Reviewed-by: Daniel Latypov <dlatypov@xxxxxxxxxx>
Sorry, some minor nits I forgot to include in my previous mail:
minor nit: perhaps KUNIT_EXPECT_TRUE(...)
I don't think most people would be confused by this, but if we send a
v3, might be good to be more precise.
>
> Create a new set of macros that take care of NULL checks.
>
> Signed-off-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx>
> ---
> include/kunit/test.h | 88 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 88 insertions(+)
>
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index 00b9ff7783ab..5970d3a0e4af 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -1218,6 +1218,50 @@ do { \
> fmt, \
> ##__VA_ARGS__)
>
> +/**
> + * KUNIT_EXPECT_NULL() - Expects that @ptr is null.
> + * @test: The test context object.
> + * @ptr: an arbitrary pointer.
> + *
> + * Sets an expectation that the value that @ptr evaluates to is null. This is
> + * semantically equivalent to KUNIT_EXPECT_PTR_EQ(@test, NULL, ptr).
> + * See KUNIT_EXPECT_TRUE() for more information.
> + */
> +#define KUNIT_EXPECT_NULL(test, ptr) \
> + KUNIT_EXPECT_PTR_EQ_MSG(test, \
> + (typeof(ptr))NULL, \
First point: hmm, I dropped the (typeof(ptr)) casting and didn't have
any build warnings.
So I don't think the cast is strictly necessary.
We use this value in the comparison (left == right) and when we store
them in the assert struct.
The former is fine since `myptr == NULL` is valid, and the latter is
fine since we store both left/right as `void *` in the end.
It does have the benefit of making the comparison operand more
distinct from the NULL `msg` parameter, however.
I personally would lean towards dropping it still.
A bit less to read, and it also generates less code after the
preprocessing step.
Second point: I think it might be more natural to have the NULL be the
`right` parameter.
Right now we get
Expected ((void *)0) == test, but
((void *)0) == 0000000000000000
test == 00000000a1803d18
I think it's a bit more natural for the user to see the expression
they provided first, i.e.
Expected test == ((void *)0), but
test == 00000000a1003d18
((void *)0) == 0000000000000000
If we do flip the order, let's remember to update the comments as
well, the "semantically equivalent to" bit.
