[PATCH 1/2] kunit: Do not typecheck binary assertions

[David Gow]

The use of typecheck() in KUNIT_EXPECT_EQ() and friends is causing more
problems than I think it's worth. Things like enums need to have their
values explicitly cast, and literals all need to be very precisely typed
for the code to compile.

While typechecking does have its uses, the additional overhead of having
lots of needless casts -- combined with the awkward error messages which
don't mention which types are involved -- makes tests less readable and
more difficult to write.

By removing the typecheck() call, the two arguments still need to be of
compatible types, but don't need to be of exactly the same time, which
seems a less confusing and more useful compromise.

Signed-off-by: David Gow <davi...@google.com>
---

I appreciate that this is probably a bit controversial (and, indeed, I
was a bit hesitant about sending it out myself), but after sitting on it
for a few days, I still think this is probably an improvement overall.

The second patch does fix what I think is an actual bug, though, so even
if this isn't determined to be a good idea, it (or some equivalent)
should probably go through.

Cheers,
-- David

include/kunit/test.h | 1 -
1 file changed, 1 deletion(-)

diff --git a/include/kunit/test.h b/include/kunit/test.h
index 49601c4b98b8..4c56ffcb7403 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -775,7 +775,6 @@ void kunit_do_assertion(struct kunit *test,
do { \
typeof(left) __left = (left); \
typeof(right) __right = (right); \
- ((void)__typecheck(__left, __right)); \
\
KUNIT_ASSERTION(test, \
__left op __right, \
--
2.31.1.607.g51e8a6a459-goog

[David Gow]

Currently, the KUNIT_EXPECT_STREQ() and related macros assign both
string arguments to variables of their own type (via typeof()). This
seems to be to prevent the macro argument from being evaluated multiple
times.

However, yhis doesn't work if one of these is a fixed-length character
array, rather than a character pointer, as (for example) char[16] will
always allocate a new string.

By always using 'const char*' (the type strcmp expects), we're always
just taking a pointer to the string, which works even with character
arrays.

Signed-off-by: David Gow <davi...@google.com>
---

include/kunit/test.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/kunit/test.h b/include/kunit/test.h
index 4c56ffcb7403..b68c61348121 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -1128,8 +1128,8 @@ do { \
fmt, \
...) \
do { \
- typeof(left) __left = (left); \
- typeof(right) __right = (right); \
+ const char *__left = (left); \
+ const char *__right = (right); \
\
KUNIT_ASSERTION(test, \
strcmp(__left, __right) op 0, \
--
2.31.1.607.g51e8a6a459-goog

[Daniel Latypov]

On Thu, May 6, 2021 at 10:09 PM David Gow <davi...@google.com> wrote:
>
> The use of typecheck() in KUNIT_EXPECT_EQ() and friends is causing more
> problems than I think it's worth. Things like enums need to have their
> values explicitly cast, and literals all need to be very precisely typed
> for the code to compile.
>
> While typechecking does have its uses, the additional overhead of having
> lots of needless casts -- combined with the awkward error messages which
> don't mention which types are involved -- makes tests less readable and
> more difficult to write.
>
> By removing the typecheck() call, the two arguments still need to be of
> compatible types, but don't need to be of exactly the same time, which
> seems a less confusing and more useful compromise.
>
> Signed-off-by: David Gow <davi...@google.com>
> ---
>
> I appreciate that this is probably a bit controversial (and, indeed, I
> was a bit hesitant about sending it out myself), but after sitting on it
> for a few days, I still think this is probably an improvement overall.

I'm in favor.
The absolute worst part of the status quo is that the types involved
might not get shown at all in the GCC error output!
It's an incredible pain and probably has wasted a good deal of other
people's time as well.
(Maybe clang is better in this regard).

Here's a few examples where things get a bit weird:
KUNIT_EXPECT_EQ(test, 1 + 1, 2.5);
Expected 1 + 1 == 2.5, but
1 + 1 == 2
2.5 == 2

Along similar lines:
KUNIT_EXPECT_EQ(test, 0xffffffff, ~0);
KUNIT_EXPECT_EQ(test, 0xffffffffffffffff, ~0);
KUNIT_EXPECT_EQ(test, 0xfffffffffffffffe, ~1);
KUNIT_EXPECT_EQ(test, 0xfffffffe, ~0); //fails
The failure message on the last might make one wonder how the first ones worked.
Expected 0xfffffffe == ~0, but
0xfffffffe == 4294967294
~0 == -1

Explanation: when evaluating the assertion, we compare __left/__right
directly which maintain their types.
But struct kunit_binary_assert stores them as `long long`, hence the
truncation of 2.5 to 2.

I was nervous about ~0, as it should be an int, i.e. this passes:
KUNIT_EXPECT_EQ(test, sizeof(~0), sizeof(int))
But it all works as expected, e.g. we don't have implicit narrowing
going on and causing us to say that 0xfffffffffffffffe = 0.


Stuff like
KUNIT_EXPECT_EQ(test, 0, NULL);
will compile, but with warnings
../include/kunit/test.h:805:15: warning: comparison between pointer and integer
805 | left, ==, right, \
| ^~

So I generally think that we can rely on compiler warnings to protect
us from some misuse.

Reviewed-by: Daniel Latypov <dlat...@google.com>

[Daniel Latypov]

On Thu, May 6, 2021 at 10:09 PM David Gow <davi...@google.com> wrote:
>

> Currently, the KUNIT_EXPECT_STREQ() and related macros assign both
> string arguments to variables of their own type (via typeof()). This
> seems to be to prevent the macro argument from being evaluated multiple
> times.
>
> However, yhis doesn't work if one of these is a fixed-length character

nit: if you ever send a v2 of this patch, s/yhis/this

> array, rather than a character pointer, as (for example) char[16] will
> always allocate a new string.
>
> By always using 'const char*' (the type strcmp expects), we're always
> just taking a pointer to the string, which works even with character
> arrays.
>
> Signed-off-by: David Gow <davi...@google.com>

Reviewed-by: Daniel Latypov <dlat...@google.com>

I'm very happy to see this patch.
This makes code that looks obviously correct actually work.

Somewhat tangential: there are several casts that are no longer needed
after this in the docs.
I think the following gets rid of all of them. Should it perhaps go in
a chain with this patch?
I.e. if the first one is too controversial and we want to go ahead
split this patch off from it.

diff --git a/Documentation/dev-tools/kunit/usage.rst
b/Documentation/dev-tools/kunit/usage.rst
index 650f99590df5..756747417a19 100644
--- a/Documentation/dev-tools/kunit/usage.rst
+++ b/Documentation/dev-tools/kunit/usage.rst
@@ -465,10 +465,9 @@ fictitious example for ``sha1sum(1)``

.. code-block:: c

- /* Note: the cast is to satisfy overly strict type-checking. */
#define TEST_SHA1(in, want) \
sha1sum(in, out); \
- KUNIT_EXPECT_STREQ_MSG(test, (char *)out, want,
"sha1sum(%s)", in);
+ KUNIT_EXPECT_STREQ_MSG(test, out, want, "sha1sum(%s)", in);

char out[40];
TEST_SHA1("hello world", "2aae6c35c94fcfb415dbe95f408b9ce91ee846ed");
@@ -507,7 +506,7 @@ In some cases, it can be helpful to write a
*table-driven test* instead, e.g.
};
for (i = 0; i < ARRAY_SIZE(cases); ++i) {
sha1sum(cases[i].str, out);
- KUNIT_EXPECT_STREQ_MSG(test, (char *)out, cases[i].sha1,
+ KUNIT_EXPECT_STREQ_MSG(test, out, cases[i].sha1,
"sha1sum(%s)", cases[i].str);
}

@@ -568,7 +567,7 @@ Reusing the same ``cases`` array from above, we
can write the test as a
struct sha1_test_case *test_param = (struct
sha1_test_case *)(test->param_value);

sha1sum(test_param->str, out);
- KUNIT_EXPECT_STREQ_MSG(test, (char *)out, test_param->sha1,
+ KUNIT_EXPECT_STREQ_MSG(test, out, test_param->sha1,
"sha1sum(%s)", test_param->str);

[Brendan Higgins]

On Thu, May 6, 2021 at 10:09 PM David Gow <davi...@google.com> wrote:
>

> The use of typecheck() in KUNIT_EXPECT_EQ() and friends is causing more
> problems than I think it's worth. Things like enums need to have their
> values explicitly cast, and literals all need to be very precisely typed
> for the code to compile.

nit: I have not had the typecheck() call prevent any code from
compiling, just generating warnings. I guess you can have a build set
to cause any warning to be promoted to an error; still, I think this
statement is misleading.

> While typechecking does have its uses, the additional overhead of having
> lots of needless casts -- combined with the awkward error messages which
> don't mention which types are involved -- makes tests less readable and
> more difficult to write.
>
> By removing the typecheck() call, the two arguments still need to be of
> compatible types, but don't need to be of exactly the same time, which
> seems a less confusing and more useful compromise.
>
> Signed-off-by: David Gow <davi...@google.com>

Looks good to me.

Reviewed-by: Brendan Higgins <brendan...@google.com>

> ---
>
> I appreciate that this is probably a bit controversial (and, indeed, I
> was a bit hesitant about sending it out myself), but after sitting on it
> for a few days, I still think this is probably an improvement overall.
>
> The second patch does fix what I think is an actual bug, though, so even
> if this isn't determined to be a good idea, it (or some equivalent)
> should probably go through.

I don't remember being a huge fan of the typecheck when it was asked
for either. I think I am a little bit more indifferent than you;
nevertheless, I support this change.

[Brendan Higgins]

On Thu, May 6, 2021 at 10:09 PM David Gow <davi...@google.com> wrote:
>

> Currently, the KUNIT_EXPECT_STREQ() and related macros assign both
> string arguments to variables of their own type (via typeof()). This
> seems to be to prevent the macro argument from being evaluated multiple
> times.
>
> However, yhis doesn't work if one of these is a fixed-length character
> array, rather than a character pointer, as (for example) char[16] will
> always allocate a new string.
>
> By always using 'const char*' (the type strcmp expects), we're always
> just taking a pointer to the string, which works even with character
> arrays.
>
> Signed-off-by: David Gow <davi...@google.com>

Aside from the nit that Daniel pointed out, this looks good to me.

Reviewed-by: Brendan Higgins <brendan...@google.com>

[David Gow]

On Sat, May 8, 2021 at 4:05 AM 'Brendan Higgins' via KUnit Development
<kuni...@googlegroups.com> wrote:
>
> On Thu, May 6, 2021 at 10:09 PM David Gow <davi...@google.com> wrote:
> >
> > The use of typecheck() in KUNIT_EXPECT_EQ() and friends is causing more
> > problems than I think it's worth. Things like enums need to have their
> > values explicitly cast, and literals all need to be very precisely typed
> > for the code to compile.
>
> nit: I have not had the typecheck() call prevent any code from
> compiling, just generating warnings. I guess you can have a build set
> to cause any warning to be promoted to an error; still, I think this
> statement is misleading.
>

Whoops -- it was the issue in patch 2 that was causing the error. This
is indeed just a warning.

I'll send out a second version with a more accurate description next
week, assuming no further issues appear.

> > While typechecking does have its uses, the additional overhead of having
> > lots of needless casts -- combined with the awkward error messages which
> > don't mention which types are involved -- makes tests less readable and
> > more difficult to write.
> >
> > By removing the typecheck() call, the two arguments still need to be of
> > compatible types, but don't need to be of exactly the same time, which
> > seems a less confusing and more useful compromise.
> >
> > Signed-off-by: David Gow <davi...@google.com>
>
> Looks good to me.
>
> Reviewed-by: Brendan Higgins <brendan...@google.com>
>
> > ---
> >
> > I appreciate that this is probably a bit controversial (and, indeed, I
> > was a bit hesitant about sending it out myself), but after sitting on it
> > for a few days, I still think this is probably an improvement overall.
> >
> > The second patch does fix what I think is an actual bug, though, so even
> > if this isn't determined to be a good idea, it (or some equivalent)
> > should probably go through.
>
> I don't remember being a huge fan of the typecheck when it was asked
> for either. I think I am a little bit more indifferent than you;
> nevertheless, I support this change.
>

> --
> You received this message because you are subscribed to the Google Groups "KUnit Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/CAFd5g44bot7S-Ya7s7QxnKfXHcy8WxUcNPsZuw_qWMaAQbqLCg%40mail.gmail.com.

[David Gow]

The use of typecheck() in KUNIT_EXPECT_EQ() and friends is causing more
problems than I think it's worth. Things like enums need to have their
values explicitly cast, and literals all need to be very precisely

typed, else a large warning will be printed.

While typechecking does have its uses, the additional overhead of having
lots of needless casts -- combined with the awkward error messages which
don't mention which types are involved -- makes tests less readable and
more difficult to write.

By removing the typecheck() call, the two arguments still need to be of
compatible types, but don't need to be of exactly the same time, which
seems a less confusing and more useful compromise.

Signed-off-by: David Gow <davi...@google.com>

Reviewed-by: Daniel Latypov <dlat...@google.com>

Reviewed-by: Brendan Higgins <brendan...@google.com>
---

Changes since v1:
https://lore.kernel.org/linux-kselftest/20210507050908.1...@google.com/
- Tidy up the patch description to note that a warning was being
produced, not an error.
- Add additional patches to remove many of the now unnecessary casts.

include/kunit/test.h | 1 -
1 file changed, 1 deletion(-)

diff --git a/include/kunit/test.h b/include/kunit/test.h
index 49601c4b98b8..4c56ffcb7403 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h

@@ -775,7 +775,6 @@ void kunit_do_assertion(struct kunit *test,
do { \
typeof(left) __left = (left); \
typeof(right) __right = (right); \
- ((void)__typecheck(__left, __right)); \
\
KUNIT_ASSERTION(test, \
__left op __right, \
--

2.31.1.751.gd2f1c929bd-goog

[David Gow]

Currently, the KUNIT_EXPECT_STREQ() and related macros assign both
string arguments to variables of their own type (via typeof()). This
seems to be to prevent the macro argument from being evaluated multiple
times.

However, this doesn't work if one of these is a fixed-length character

array, rather than a character pointer, as (for example) char[16] will
always allocate a new string.

By always using 'const char*' (the type strcmp expects), we're always
just taking a pointer to the string, which works even with character
arrays.

Signed-off-by: David Gow <davi...@google.com>
Reviewed-by: Daniel Latypov <dlat...@google.com>
Reviewed-by: Brendan Higgins <brendan...@google.com>
---

include/kunit/test.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

Changes since v1:
https://lore.kernel.org/linux-kselftest/20210507050908.1...@google.com/

- Fix a typo in the description ('yhis' -> 'this').


diff --git a/include/kunit/test.h b/include/kunit/test.h
index 4c56ffcb7403..b68c61348121 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h

@@ -1128,8 +1128,8 @@ do { \
fmt, \
...) \
do { \
- typeof(left) __left = (left); \
- typeof(right) __right = (right); \
+ const char *__left = (left); \
+ const char *__right = (right); \
\
KUNIT_ASSERTION(test, \
strcmp(__left, __right) op 0, \
--

2.31.1.751.gd2f1c929bd-goog

[David Gow]

As the type checking is no longer excessively strict, get rid of the
unsightly (char*) casts -- and comment discussing them -- from the KUnit
usage page.

Signed-off-by: David Gow <davi...@google.com>
---

Documentation/dev-tools/kunit/usage.rst | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)

2.31.1.751.gd2f1c929bd-goog

[David Gow]

With some of the stricter type checking in KUnit's EXPECT macros
removed, several casts in property-entry-test are no longer required.

Remove the unnecessary casts, making the conditions clearer.

Signed-off-by: David Gow <davi...@google.com>
---

This should be a no-op functionality wise, and while it depends on the
first couple of patches in this series, it's otherwise independent from
the others. I think this makes the test more readable, but if you
particularly dislike it, I'm happy to drop it.

drivers/base/test/property-entry-test.c | 56 ++++++++++++-------------
1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/drivers/base/test/property-entry-test.c b/drivers/base/test/property-entry-test.c
index 1106fedcceed..6071d5bc128c 100644
--- a/drivers/base/test/property-entry-test.c
+++ b/drivers/base/test/property-entry-test.c
@@ -32,11 +32,11 @@ static void pe_test_uints(struct kunit *test)

error = fwnode_property_read_u8(node, "prop-u8", &val_u8);
KUNIT_EXPECT_EQ(test, error, 0);
- KUNIT_EXPECT_EQ(test, (int)val_u8, 8);
+ KUNIT_EXPECT_EQ(test, val_u8, 8);

error = fwnode_property_read_u8_array(node, "prop-u8", array_u8, 1);
KUNIT_EXPECT_EQ(test, error, 0);
- KUNIT_EXPECT_EQ(test, (int)array_u8[0], 8);
+ KUNIT_EXPECT_EQ(test, array_u8[0], 8);

error = fwnode_property_read_u8_array(node, "prop-u8", array_u8, 2);
KUNIT_EXPECT_NE(test, error, 0);
@@ -49,14 +49,14 @@ static void pe_test_uints(struct kunit *test)

error = fwnode_property_read_u16(node, "prop-u16", &val_u16);
KUNIT_EXPECT_EQ(test, error, 0);
- KUNIT_EXPECT_EQ(test, (int)val_u16, 16);
+ KUNIT_EXPECT_EQ(test, val_u16, 16);

error = fwnode_property_count_u16(node, "prop-u16");
KUNIT_EXPECT_EQ(test, error, 1);

error = fwnode_property_read_u16_array(node, "prop-u16", array_u16, 1);
KUNIT_EXPECT_EQ(test, error, 0);
- KUNIT_EXPECT_EQ(test, (int)array_u16[0], 16);
+ KUNIT_EXPECT_EQ(test, array_u16[0], 16);

error = fwnode_property_read_u16_array(node, "prop-u16", array_u16, 2);
KUNIT_EXPECT_NE(test, error, 0);
@@ -69,14 +69,14 @@ static void pe_test_uints(struct kunit *test)

error = fwnode_property_read_u32(node, "prop-u32", &val_u32);
KUNIT_EXPECT_EQ(test, error, 0);
- KUNIT_EXPECT_EQ(test, (int)val_u32, 32);
+ KUNIT_EXPECT_EQ(test, val_u32, 32);

error = fwnode_property_count_u32(node, "prop-u32");
KUNIT_EXPECT_EQ(test, error, 1);

error = fwnode_property_read_u32_array(node, "prop-u32", array_u32, 1);
KUNIT_EXPECT_EQ(test, error, 0);
- KUNIT_EXPECT_EQ(test, (int)array_u32[0], 32);
+ KUNIT_EXPECT_EQ(test, array_u32[0], 32);

error = fwnode_property_read_u32_array(node, "prop-u32", array_u32, 2);
KUNIT_EXPECT_NE(test, error, 0);
@@ -89,14 +89,14 @@ static void pe_test_uints(struct kunit *test)

error = fwnode_property_read_u64(node, "prop-u64", &val_u64);
KUNIT_EXPECT_EQ(test, error, 0);
- KUNIT_EXPECT_EQ(test, (int)val_u64, 64);
+ KUNIT_EXPECT_EQ(test, val_u64, 64);

error = fwnode_property_count_u64(node, "prop-u64");
KUNIT_EXPECT_EQ(test, error, 1);

error = fwnode_property_read_u64_array(node, "prop-u64", array_u64, 1);
KUNIT_EXPECT_EQ(test, error, 0);
- KUNIT_EXPECT_EQ(test, (int)array_u64[0], 64);
+ KUNIT_EXPECT_EQ(test, array_u64[0], 64);

error = fwnode_property_read_u64_array(node, "prop-u64", array_u64, 2);
KUNIT_EXPECT_NE(test, error, 0);
@@ -140,19 +140,19 @@ static void pe_test_uint_arrays(struct kunit *test)

error = fwnode_property_read_u8(node, "prop-u8", &val_u8);
KUNIT_EXPECT_EQ(test, error, 0);
- KUNIT_EXPECT_EQ(test, (int)val_u8, 8);
+ KUNIT_EXPECT_EQ(test, val_u8, 8);

error = fwnode_property_count_u8(node, "prop-u8");
KUNIT_EXPECT_EQ(test, error, 10);

error = fwnode_property_read_u8_array(node, "prop-u8", array_u8, 1);
KUNIT_EXPECT_EQ(test, error, 0);
- KUNIT_EXPECT_EQ(test, (int)array_u8[0], 8);
+ KUNIT_EXPECT_EQ(test, array_u8[0], 8);

error = fwnode_property_read_u8_array(node, "prop-u8", array_u8, 2);
KUNIT_EXPECT_EQ(test, error, 0);
- KUNIT_EXPECT_EQ(test, (int)array_u8[0], 8);
- KUNIT_EXPECT_EQ(test, (int)array_u8[1], 9);
+ KUNIT_EXPECT_EQ(test, array_u8[0], 8);
+ KUNIT_EXPECT_EQ(test, array_u8[1], 9);

error = fwnode_property_read_u8_array(node, "prop-u8", array_u8, 17);
KUNIT_EXPECT_NE(test, error, 0);
@@ -165,19 +165,19 @@ static void pe_test_uint_arrays(struct kunit *test)

error = fwnode_property_read_u16(node, "prop-u16", &val_u16);
KUNIT_EXPECT_EQ(test, error, 0);
- KUNIT_EXPECT_EQ(test, (int)val_u16, 16);
+ KUNIT_EXPECT_EQ(test, val_u16, 16);

error = fwnode_property_count_u16(node, "prop-u16");
KUNIT_EXPECT_EQ(test, error, 10);

error = fwnode_property_read_u16_array(node, "prop-u16", array_u16, 1);
KUNIT_EXPECT_EQ(test, error, 0);
- KUNIT_EXPECT_EQ(test, (int)array_u16[0], 16);
+ KUNIT_EXPECT_EQ(test, array_u16[0], 16);

error = fwnode_property_read_u16_array(node, "prop-u16", array_u16, 2);
KUNIT_EXPECT_EQ(test, error, 0);
- KUNIT_EXPECT_EQ(test, (int)array_u16[0], 16);
- KUNIT_EXPECT_EQ(test, (int)array_u16[1], 17);
+ KUNIT_EXPECT_EQ(test, array_u16[0], 16);
+ KUNIT_EXPECT_EQ(test, array_u16[1], 17);

error = fwnode_property_read_u16_array(node, "prop-u16", array_u16, 17);
KUNIT_EXPECT_NE(test, error, 0);
@@ -190,19 +190,19 @@ static void pe_test_uint_arrays(struct kunit *test)

error = fwnode_property_read_u32(node, "prop-u32", &val_u32);
KUNIT_EXPECT_EQ(test, error, 0);
- KUNIT_EXPECT_EQ(test, (int)val_u32, 32);
+ KUNIT_EXPECT_EQ(test, val_u32, 32);

error = fwnode_property_count_u32(node, "prop-u32");
KUNIT_EXPECT_EQ(test, error, 10);

error = fwnode_property_read_u32_array(node, "prop-u32", array_u32, 1);
KUNIT_EXPECT_EQ(test, error, 0);
- KUNIT_EXPECT_EQ(test, (int)array_u32[0], 32);
+ KUNIT_EXPECT_EQ(test, array_u32[0], 32);

error = fwnode_property_read_u32_array(node, "prop-u32", array_u32, 2);
KUNIT_EXPECT_EQ(test, error, 0);
- KUNIT_EXPECT_EQ(test, (int)array_u32[0], 32);
- KUNIT_EXPECT_EQ(test, (int)array_u32[1], 33);
+ KUNIT_EXPECT_EQ(test, array_u32[0], 32);
+ KUNIT_EXPECT_EQ(test, array_u32[1], 33);

error = fwnode_property_read_u32_array(node, "prop-u32", array_u32, 17);
KUNIT_EXPECT_NE(test, error, 0);
@@ -215,19 +215,19 @@ static void pe_test_uint_arrays(struct kunit *test)

error = fwnode_property_read_u64(node, "prop-u64", &val_u64);
KUNIT_EXPECT_EQ(test, error, 0);
- KUNIT_EXPECT_EQ(test, (int)val_u64, 64);
+ KUNIT_EXPECT_EQ(test, val_u64, 64);

error = fwnode_property_count_u64(node, "prop-u64");
KUNIT_EXPECT_EQ(test, error, 10);

error = fwnode_property_read_u64_array(node, "prop-u64", array_u64, 1);
KUNIT_EXPECT_EQ(test, error, 0);
- KUNIT_EXPECT_EQ(test, (int)array_u64[0], 64);
+ KUNIT_EXPECT_EQ(test, array_u64[0], 64);

error = fwnode_property_read_u64_array(node, "prop-u64", array_u64, 2);
KUNIT_EXPECT_EQ(test, error, 0);
- KUNIT_EXPECT_EQ(test, (int)array_u64[0], 64);
- KUNIT_EXPECT_EQ(test, (int)array_u64[1], 65);
+ KUNIT_EXPECT_EQ(test, array_u64[0], 64);
+ KUNIT_EXPECT_EQ(test, array_u64[1], 65);

error = fwnode_property_read_u64_array(node, "prop-u64", array_u64, 17);
KUNIT_EXPECT_NE(test, error, 0);
@@ -358,13 +358,13 @@ static void pe_test_move_inline_u8(struct kunit *test)

KUNIT_EXPECT_TRUE(test, copy[0].is_inline);
data_ptr = (u8 *)&copy[0].value;
- KUNIT_EXPECT_EQ(test, (int)data_ptr[0], 1);
- KUNIT_EXPECT_EQ(test, (int)data_ptr[1], 2);
+ KUNIT_EXPECT_EQ(test, data_ptr[0], 1);
+ KUNIT_EXPECT_EQ(test, data_ptr[1], 2);

KUNIT_EXPECT_FALSE(test, copy[1].is_inline);
data_ptr = copy[1].pointer;
- KUNIT_EXPECT_EQ(test, (int)data_ptr[0], 5);
- KUNIT_EXPECT_EQ(test, (int)data_ptr[1], 6);
+ KUNIT_EXPECT_EQ(test, data_ptr[0], 5);
+ KUNIT_EXPECT_EQ(test, data_ptr[1], 6);

property_entries_free(copy);
}
--
2.31.1.751.gd2f1c929bd-goog

[David Gow]

KUnit's EXPECT macros no longer typecheck as stringently, so casting the
result of strcmp() is now unnecessary.

Signed-off-by: David Gow <davi...@google.com>
---
This should be a no-op functionality wise, and while it depends on the
first couple of patches in this series, it's otherwise independent from
the others. I think this makes the test more readable, but if you
particularly dislike it, I'm happy to drop it.

drivers/iio/test/iio-test-format.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/test/iio-test-format.c b/drivers/iio/test/iio-test-format.c
index 55a0cfe9181d..f1e951eddb43 100644
--- a/drivers/iio/test/iio-test-format.c
+++ b/drivers/iio/test/iio-test-format.c
@@ -8,7 +8,7 @@
#include <linux/iio/iio.h>

#define IIO_TEST_FORMAT_EXPECT_EQ(_test, _buf, _ret, _val) do { \
- KUNIT_EXPECT_EQ(_test, (int)strlen(_buf), _ret); \
+ KUNIT_EXPECT_EQ(_test, strlen(_buf), _ret); \
KUNIT_EXPECT_STREQ(_test, (_buf), (_val)); \
} while (0)

--
2.31.1.751.gd2f1c929bd-goog

[David Gow]

With KUnit's EXPECT macros no longer typechecking arguments as strictly,
get rid of a number of now unnecessary casts.

Signed-off-by: David Gow <davi...@google.com>
---
This should be a no-op functionality wise, and while it depends on the
first couple of patches in this series, it's otherwise independent from
the others. I think this makes the test more readable, but if you
particularly dislike it, I'm happy to drop it.

drivers/mmc/host/sdhci-of-aspeed-test.c | 34 ++++++++++++-------------
1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-aspeed-test.c b/drivers/mmc/host/sdhci-of-aspeed-test.c
index bb67d159b7d8..1ed4f86291f2 100644
--- a/drivers/mmc/host/sdhci-of-aspeed-test.c
+++ b/drivers/mmc/host/sdhci-of-aspeed-test.c
@@ -26,23 +26,23 @@ static void aspeed_sdhci_phase_ddr52(struct kunit *test)
KUNIT_EXPECT_EQ(test, 15,
aspeed_sdhci_phase_to_tap(NULL, rate, 25));

- KUNIT_EXPECT_EQ(test, (int)ASPEED_SDHCI_TAP_PARAM_INVERT_CLK | 0,
+ KUNIT_EXPECT_EQ(test, ASPEED_SDHCI_TAP_PARAM_INVERT_CLK | 0,
aspeed_sdhci_phase_to_tap(NULL, rate, 180));
- KUNIT_EXPECT_EQ(test, (int)ASPEED_SDHCI_TAP_PARAM_INVERT_CLK | 0,
+ KUNIT_EXPECT_EQ(test, ASPEED_SDHCI_TAP_PARAM_INVERT_CLK | 0,
aspeed_sdhci_phase_to_tap(NULL, rate, 181));
- KUNIT_EXPECT_EQ(test, (int)ASPEED_SDHCI_TAP_PARAM_INVERT_CLK | 1,
+ KUNIT_EXPECT_EQ(test, ASPEED_SDHCI_TAP_PARAM_INVERT_CLK | 1,
aspeed_sdhci_phase_to_tap(NULL, rate, 182));
- KUNIT_EXPECT_EQ(test, (int)ASPEED_SDHCI_TAP_PARAM_INVERT_CLK | 1,
+ KUNIT_EXPECT_EQ(test, ASPEED_SDHCI_TAP_PARAM_INVERT_CLK | 1,
aspeed_sdhci_phase_to_tap(NULL, rate, 183));
- KUNIT_EXPECT_EQ(test, (int)ASPEED_SDHCI_TAP_PARAM_INVERT_CLK | 2,
+ KUNIT_EXPECT_EQ(test, ASPEED_SDHCI_TAP_PARAM_INVERT_CLK | 2,
aspeed_sdhci_phase_to_tap(NULL, rate, 184));
- KUNIT_EXPECT_EQ(test, (int)ASPEED_SDHCI_TAP_PARAM_INVERT_CLK | 3,
+ KUNIT_EXPECT_EQ(test, ASPEED_SDHCI_TAP_PARAM_INVERT_CLK | 3,
aspeed_sdhci_phase_to_tap(NULL, rate, 185));
- KUNIT_EXPECT_EQ(test, (int)ASPEED_SDHCI_TAP_PARAM_INVERT_CLK | 14,
+ KUNIT_EXPECT_EQ(test, ASPEED_SDHCI_TAP_PARAM_INVERT_CLK | 14,
aspeed_sdhci_phase_to_tap(NULL, rate, 203));
- KUNIT_EXPECT_EQ(test, (int)ASPEED_SDHCI_TAP_PARAM_INVERT_CLK | 15,
+ KUNIT_EXPECT_EQ(test, ASPEED_SDHCI_TAP_PARAM_INVERT_CLK | 15,
aspeed_sdhci_phase_to_tap(NULL, rate, 204));
- KUNIT_EXPECT_EQ(test, (int)ASPEED_SDHCI_TAP_PARAM_INVERT_CLK | 15,
+ KUNIT_EXPECT_EQ(test, ASPEED_SDHCI_TAP_PARAM_INVERT_CLK | 15,
aspeed_sdhci_phase_to_tap(NULL, rate, 205));
}

@@ -67,21 +67,21 @@ static void aspeed_sdhci_phase_hs200(struct kunit *test)
KUNIT_EXPECT_EQ(test, 15,
aspeed_sdhci_phase_to_tap(NULL, rate, 96));

- KUNIT_EXPECT_EQ(test, (int)ASPEED_SDHCI_TAP_PARAM_INVERT_CLK,
+ KUNIT_EXPECT_EQ(test, ASPEED_SDHCI_TAP_PARAM_INVERT_CLK,
aspeed_sdhci_phase_to_tap(NULL, rate, 180));
- KUNIT_EXPECT_EQ(test, (int)ASPEED_SDHCI_TAP_PARAM_INVERT_CLK,
+ KUNIT_EXPECT_EQ(test, ASPEED_SDHCI_TAP_PARAM_INVERT_CLK,
aspeed_sdhci_phase_to_tap(NULL, rate, 185));
- KUNIT_EXPECT_EQ(test, (int)ASPEED_SDHCI_TAP_PARAM_INVERT_CLK | 1,
+ KUNIT_EXPECT_EQ(test, ASPEED_SDHCI_TAP_PARAM_INVERT_CLK | 1,
aspeed_sdhci_phase_to_tap(NULL, rate, 186));
- KUNIT_EXPECT_EQ(test, (int)ASPEED_SDHCI_TAP_PARAM_INVERT_CLK | 1,
+ KUNIT_EXPECT_EQ(test, ASPEED_SDHCI_TAP_PARAM_INVERT_CLK | 1,
aspeed_sdhci_phase_to_tap(NULL, rate, 187));
- KUNIT_EXPECT_EQ(test, (int)ASPEED_SDHCI_TAP_PARAM_INVERT_CLK | 14,
+ KUNIT_EXPECT_EQ(test, ASPEED_SDHCI_TAP_PARAM_INVERT_CLK | 14,
aspeed_sdhci_phase_to_tap(NULL, rate, 269));
- KUNIT_EXPECT_EQ(test, (int)ASPEED_SDHCI_TAP_PARAM_INVERT_CLK | 15,
+ KUNIT_EXPECT_EQ(test, ASPEED_SDHCI_TAP_PARAM_INVERT_CLK | 15,
aspeed_sdhci_phase_to_tap(NULL, rate, 270));
- KUNIT_EXPECT_EQ(test, (int)ASPEED_SDHCI_TAP_PARAM_INVERT_CLK | 15,
+ KUNIT_EXPECT_EQ(test, ASPEED_SDHCI_TAP_PARAM_INVERT_CLK | 15,
aspeed_sdhci_phase_to_tap(NULL, rate, 271));
- KUNIT_EXPECT_EQ(test, (int)ASPEED_SDHCI_TAP_PARAM_INVERT_CLK | 15,
+ KUNIT_EXPECT_EQ(test, ASPEED_SDHCI_TAP_PARAM_INVERT_CLK | 15,
aspeed_sdhci_phase_to_tap(NULL, rate, 276));
}

--
2.31.1.751.gd2f1c929bd-goog

[David Gow]

With some of the stricter type checking in KUnit's EXPECT macros

removed, several casts in the thunderbolt KUnit tests are no longer

required.

Remove the unnecessary casts, making the conditions clearer.

Signed-off-by: David Gow <davi...@google.com>
---
This should be a no-op functionality wise, and while it depends on the
first couple of patches in this series, it's otherwise independent from
the others. I think this makes the test more readable, but if you
particularly dislike it, I'm happy to drop it.

drivers/thunderbolt/test.c | 152 ++++++++++++++++---------------------
1 file changed, 65 insertions(+), 87 deletions(-)

diff --git a/drivers/thunderbolt/test.c b/drivers/thunderbolt/test.c
index 5ff5a03bc9ce..247dc9f4757e 100644
--- a/drivers/thunderbolt/test.c
+++ b/drivers/thunderbolt/test.c
@@ -384,20 +384,18 @@ static void tb_test_path_single_hop_walk(struct kunit *test)
KUNIT_EXPECT_TRUE(test, i < ARRAY_SIZE(test_data));
KUNIT_EXPECT_EQ(test, tb_route(p->sw), test_data[i].route);
KUNIT_EXPECT_EQ(test, p->port, test_data[i].port);
- KUNIT_EXPECT_EQ(test, (enum tb_port_type)p->config.type,
- test_data[i].type);
+ KUNIT_EXPECT_EQ(test, p->config.type, test_data[i].type);
i++;
}

- KUNIT_EXPECT_EQ(test, i, (int)ARRAY_SIZE(test_data));
+ KUNIT_EXPECT_EQ(test, i, ARRAY_SIZE(test_data));

i = ARRAY_SIZE(test_data) - 1;
tb_for_each_port_on_path(dst_port, src_port, p) {
KUNIT_EXPECT_TRUE(test, i < ARRAY_SIZE(test_data));
KUNIT_EXPECT_EQ(test, tb_route(p->sw), test_data[i].route);
KUNIT_EXPECT_EQ(test, p->port, test_data[i].port);
- KUNIT_EXPECT_EQ(test, (enum tb_port_type)p->config.type,
- test_data[i].type);
+ KUNIT_EXPECT_EQ(test, p->config.type, test_data[i].type);
i--;
}

@@ -443,20 +441,18 @@ static void tb_test_path_daisy_chain_walk(struct kunit *test)
KUNIT_EXPECT_TRUE(test, i < ARRAY_SIZE(test_data));
KUNIT_EXPECT_EQ(test, tb_route(p->sw), test_data[i].route);
KUNIT_EXPECT_EQ(test, p->port, test_data[i].port);
- KUNIT_EXPECT_EQ(test, (enum tb_port_type)p->config.type,
- test_data[i].type);
+ KUNIT_EXPECT_EQ(test, p->config.type, test_data[i].type);
i++;
}

- KUNIT_EXPECT_EQ(test, i, (int)ARRAY_SIZE(test_data));
+ KUNIT_EXPECT_EQ(test, i, ARRAY_SIZE(test_data));

i = ARRAY_SIZE(test_data) - 1;
tb_for_each_port_on_path(dst_port, src_port, p) {
KUNIT_EXPECT_TRUE(test, i < ARRAY_SIZE(test_data));
KUNIT_EXPECT_EQ(test, tb_route(p->sw), test_data[i].route);
KUNIT_EXPECT_EQ(test, p->port, test_data[i].port);
- KUNIT_EXPECT_EQ(test, (enum tb_port_type)p->config.type,
- test_data[i].type);
+ KUNIT_EXPECT_EQ(test, p->config.type, test_data[i].type);
i--;
}

@@ -506,20 +502,18 @@ static void tb_test_path_simple_tree_walk(struct kunit *test)
KUNIT_EXPECT_TRUE(test, i < ARRAY_SIZE(test_data));
KUNIT_EXPECT_EQ(test, tb_route(p->sw), test_data[i].route);
KUNIT_EXPECT_EQ(test, p->port, test_data[i].port);
- KUNIT_EXPECT_EQ(test, (enum tb_port_type)p->config.type,
- test_data[i].type);
+ KUNIT_EXPECT_EQ(test, p->config.type, test_data[i].type);
i++;
}

- KUNIT_EXPECT_EQ(test, i, (int)ARRAY_SIZE(test_data));
+ KUNIT_EXPECT_EQ(test, i, ARRAY_SIZE(test_data));

i = ARRAY_SIZE(test_data) - 1;
tb_for_each_port_on_path(dst_port, src_port, p) {
KUNIT_EXPECT_TRUE(test, i < ARRAY_SIZE(test_data));
KUNIT_EXPECT_EQ(test, tb_route(p->sw), test_data[i].route);
KUNIT_EXPECT_EQ(test, p->port, test_data[i].port);
- KUNIT_EXPECT_EQ(test, (enum tb_port_type)p->config.type,
- test_data[i].type);
+ KUNIT_EXPECT_EQ(test, p->config.type, test_data[i].type);
i--;
}

@@ -590,20 +584,18 @@ static void tb_test_path_complex_tree_walk(struct kunit *test)
KUNIT_EXPECT_TRUE(test, i < ARRAY_SIZE(test_data));
KUNIT_EXPECT_EQ(test, tb_route(p->sw), test_data[i].route);
KUNIT_EXPECT_EQ(test, p->port, test_data[i].port);
- KUNIT_EXPECT_EQ(test, (enum tb_port_type)p->config.type,
- test_data[i].type);
+ KUNIT_EXPECT_EQ(test, p->config.type, test_data[i].type);
i++;
}

- KUNIT_EXPECT_EQ(test, i, (int)ARRAY_SIZE(test_data));
+ KUNIT_EXPECT_EQ(test, i, ARRAY_SIZE(test_data));

i = ARRAY_SIZE(test_data) - 1;
tb_for_each_port_on_path(dst_port, src_port, p) {
KUNIT_EXPECT_TRUE(test, i < ARRAY_SIZE(test_data));
KUNIT_EXPECT_EQ(test, tb_route(p->sw), test_data[i].route);
KUNIT_EXPECT_EQ(test, p->port, test_data[i].port);
- KUNIT_EXPECT_EQ(test, (enum tb_port_type)p->config.type,
- test_data[i].type);
+ KUNIT_EXPECT_EQ(test, p->config.type, test_data[i].type);
i--;
}

@@ -693,20 +685,18 @@ static void tb_test_path_max_length_walk(struct kunit *test)
KUNIT_EXPECT_TRUE(test, i < ARRAY_SIZE(test_data));
KUNIT_EXPECT_EQ(test, tb_route(p->sw), test_data[i].route);
KUNIT_EXPECT_EQ(test, p->port, test_data[i].port);
- KUNIT_EXPECT_EQ(test, (enum tb_port_type)p->config.type,
- test_data[i].type);
+ KUNIT_EXPECT_EQ(test, p->config.type, test_data[i].type);
i++;
}

- KUNIT_EXPECT_EQ(test, i, (int)ARRAY_SIZE(test_data));
+ KUNIT_EXPECT_EQ(test, i, ARRAY_SIZE(test_data));

i = ARRAY_SIZE(test_data) - 1;
tb_for_each_port_on_path(dst_port, src_port, p) {
KUNIT_EXPECT_TRUE(test, i < ARRAY_SIZE(test_data));
KUNIT_EXPECT_EQ(test, tb_route(p->sw), test_data[i].route);
KUNIT_EXPECT_EQ(test, p->port, test_data[i].port);
- KUNIT_EXPECT_EQ(test, (enum tb_port_type)p->config.type,
- test_data[i].type);
+ KUNIT_EXPECT_EQ(test, p->config.type, test_data[i].type);
i--;
}

@@ -780,7 +770,7 @@ static void tb_test_path_not_bonded_lane0(struct kunit *test)

path = tb_path_alloc(NULL, down, 8, up, 8, 0, "PCIe Down");
KUNIT_ASSERT_TRUE(test, path != NULL);
- KUNIT_ASSERT_EQ(test, path->path_length, (int)ARRAY_SIZE(test_data));
+ KUNIT_ASSERT_EQ(test, path->path_length, ARRAY_SIZE(test_data));
for (i = 0; i < ARRAY_SIZE(test_data); i++) {
const struct tb_port *in_port, *out_port;

@@ -789,12 +779,10 @@ static void tb_test_path_not_bonded_lane0(struct kunit *test)

KUNIT_EXPECT_EQ(test, tb_route(in_port->sw), test_data[i].route);
KUNIT_EXPECT_EQ(test, in_port->port, test_data[i].in_port);
- KUNIT_EXPECT_EQ(test, (enum tb_port_type)in_port->config.type,
- test_data[i].in_type);
+ KUNIT_EXPECT_EQ(test, in_port->config.type, test_data[i].in_type);
KUNIT_EXPECT_EQ(test, tb_route(out_port->sw), test_data[i].route);
KUNIT_EXPECT_EQ(test, out_port->port, test_data[i].out_port);
- KUNIT_EXPECT_EQ(test, (enum tb_port_type)out_port->config.type,
- test_data[i].out_type);
+ KUNIT_EXPECT_EQ(test, out_port->config.type, test_data[i].out_type);
}
tb_path_free(path);
}
@@ -842,7 +830,7 @@ static void tb_test_path_not_bonded_lane1(struct kunit *test)

path = tb_path_alloc(NULL, in, 9, out, 9, 1, "Video");
KUNIT_ASSERT_TRUE(test, path != NULL);
- KUNIT_ASSERT_EQ(test, path->path_length, (int)ARRAY_SIZE(test_data));
+ KUNIT_ASSERT_EQ(test, path->path_length, ARRAY_SIZE(test_data));
for (i = 0; i < ARRAY_SIZE(test_data); i++) {
const struct tb_port *in_port, *out_port;

@@ -851,12 +839,10 @@ static void tb_test_path_not_bonded_lane1(struct kunit *test)

KUNIT_EXPECT_EQ(test, tb_route(in_port->sw), test_data[i].route);
KUNIT_EXPECT_EQ(test, in_port->port, test_data[i].in_port);
- KUNIT_EXPECT_EQ(test, (enum tb_port_type)in_port->config.type,
- test_data[i].in_type);
+ KUNIT_EXPECT_EQ(test, in_port->config.type, test_data[i].in_type);
KUNIT_EXPECT_EQ(test, tb_route(out_port->sw), test_data[i].route);
KUNIT_EXPECT_EQ(test, out_port->port, test_data[i].out_port);
- KUNIT_EXPECT_EQ(test, (enum tb_port_type)out_port->config.type,
- test_data[i].out_type);
+ KUNIT_EXPECT_EQ(test, out_port->config.type, test_data[i].out_type);
}
tb_path_free(path);
}
@@ -922,7 +908,7 @@ static void tb_test_path_not_bonded_lane1_chain(struct kunit *test)

path = tb_path_alloc(NULL, in, 9, out, 9, 1, "Video");
KUNIT_ASSERT_TRUE(test, path != NULL);
- KUNIT_ASSERT_EQ(test, path->path_length, (int)ARRAY_SIZE(test_data));
+ KUNIT_ASSERT_EQ(test, path->path_length, ARRAY_SIZE(test_data));
for (i = 0; i < ARRAY_SIZE(test_data); i++) {
const struct tb_port *in_port, *out_port;

@@ -931,12 +917,10 @@ static void tb_test_path_not_bonded_lane1_chain(struct kunit *test)

KUNIT_EXPECT_EQ(test, tb_route(in_port->sw), test_data[i].route);
KUNIT_EXPECT_EQ(test, in_port->port, test_data[i].in_port);
- KUNIT_EXPECT_EQ(test, (enum tb_port_type)in_port->config.type,
- test_data[i].in_type);
+ KUNIT_EXPECT_EQ(test, in_port->config.type, test_data[i].in_type);
KUNIT_EXPECT_EQ(test, tb_route(out_port->sw), test_data[i].route);
KUNIT_EXPECT_EQ(test, out_port->port, test_data[i].out_port);
- KUNIT_EXPECT_EQ(test, (enum tb_port_type)out_port->config.type,
- test_data[i].out_type);
+ KUNIT_EXPECT_EQ(test, out_port->config.type, test_data[i].out_type);
}
tb_path_free(path);
}
@@ -1002,7 +986,7 @@ static void tb_test_path_not_bonded_lane1_chain_reverse(struct kunit *test)

path = tb_path_alloc(NULL, in, 9, out, 9, 1, "Video");
KUNIT_ASSERT_TRUE(test, path != NULL);
- KUNIT_ASSERT_EQ(test, path->path_length, (int)ARRAY_SIZE(test_data));
+ KUNIT_ASSERT_EQ(test, path->path_length, ARRAY_SIZE(test_data));
for (i = 0; i < ARRAY_SIZE(test_data); i++) {
const struct tb_port *in_port, *out_port;

@@ -1011,12 +995,10 @@ static void tb_test_path_not_bonded_lane1_chain_reverse(struct kunit *test)

KUNIT_EXPECT_EQ(test, tb_route(in_port->sw), test_data[i].route);
KUNIT_EXPECT_EQ(test, in_port->port, test_data[i].in_port);
- KUNIT_EXPECT_EQ(test, (enum tb_port_type)in_port->config.type,
- test_data[i].in_type);
+ KUNIT_EXPECT_EQ(test, in_port->config.type, test_data[i].in_type);
KUNIT_EXPECT_EQ(test, tb_route(out_port->sw), test_data[i].route);
KUNIT_EXPECT_EQ(test, out_port->port, test_data[i].out_port);
- KUNIT_EXPECT_EQ(test, (enum tb_port_type)out_port->config.type,
- test_data[i].out_type);
+ KUNIT_EXPECT_EQ(test, out_port->config.type, test_data[i].out_type);
}
tb_path_free(path);
}
@@ -1094,7 +1076,7 @@ static void tb_test_path_mixed_chain(struct kunit *test)

path = tb_path_alloc(NULL, in, 9, out, 9, 1, "Video");
KUNIT_ASSERT_TRUE(test, path != NULL);
- KUNIT_ASSERT_EQ(test, path->path_length, (int)ARRAY_SIZE(test_data));
+ KUNIT_ASSERT_EQ(test, path->path_length, ARRAY_SIZE(test_data));
for (i = 0; i < ARRAY_SIZE(test_data); i++) {
const struct tb_port *in_port, *out_port;

@@ -1103,12 +1085,10 @@ static void tb_test_path_mixed_chain(struct kunit *test)

KUNIT_EXPECT_EQ(test, tb_route(in_port->sw), test_data[i].route);
KUNIT_EXPECT_EQ(test, in_port->port, test_data[i].in_port);
- KUNIT_EXPECT_EQ(test, (enum tb_port_type)in_port->config.type,
- test_data[i].in_type);
+ KUNIT_EXPECT_EQ(test, in_port->config.type, test_data[i].in_type);
KUNIT_EXPECT_EQ(test, tb_route(out_port->sw), test_data[i].route);
KUNIT_EXPECT_EQ(test, out_port->port, test_data[i].out_port);
- KUNIT_EXPECT_EQ(test, (enum tb_port_type)out_port->config.type,
- test_data[i].out_type);
+ KUNIT_EXPECT_EQ(test, out_port->config.type, test_data[i].out_type);
}
tb_path_free(path);
}
@@ -1186,7 +1166,7 @@ static void tb_test_path_mixed_chain_reverse(struct kunit *test)

path = tb_path_alloc(NULL, in, 9, out, 9, 1, "Video");
KUNIT_ASSERT_TRUE(test, path != NULL);
- KUNIT_ASSERT_EQ(test, path->path_length, (int)ARRAY_SIZE(test_data));
+ KUNIT_ASSERT_EQ(test, path->path_length, ARRAY_SIZE(test_data));
for (i = 0; i < ARRAY_SIZE(test_data); i++) {
const struct tb_port *in_port, *out_port;

@@ -1195,12 +1175,10 @@ static void tb_test_path_mixed_chain_reverse(struct kunit *test)

KUNIT_EXPECT_EQ(test, tb_route(in_port->sw), test_data[i].route);
KUNIT_EXPECT_EQ(test, in_port->port, test_data[i].in_port);
- KUNIT_EXPECT_EQ(test, (enum tb_port_type)in_port->config.type,
- test_data[i].in_type);
+ KUNIT_EXPECT_EQ(test, in_port->config.type, test_data[i].in_type);
KUNIT_EXPECT_EQ(test, tb_route(out_port->sw), test_data[i].route);
KUNIT_EXPECT_EQ(test, out_port->port, test_data[i].out_port);
- KUNIT_EXPECT_EQ(test, (enum tb_port_type)out_port->config.type,
- test_data[i].out_type);
+ KUNIT_EXPECT_EQ(test, out_port->config.type, test_data[i].out_type);
}
tb_path_free(path);
}
@@ -1230,10 +1208,10 @@ static void tb_test_tunnel_pcie(struct kunit *test)
up = &dev1->ports[9];
tunnel1 = tb_tunnel_alloc_pci(NULL, up, down);
KUNIT_ASSERT_TRUE(test, tunnel1 != NULL);
- KUNIT_EXPECT_EQ(test, tunnel1->type, (enum tb_tunnel_type)TB_TUNNEL_PCI);
+ KUNIT_EXPECT_EQ(test, tunnel1->type, TB_TUNNEL_PCI);
KUNIT_EXPECT_PTR_EQ(test, tunnel1->src_port, down);
KUNIT_EXPECT_PTR_EQ(test, tunnel1->dst_port, up);
- KUNIT_ASSERT_EQ(test, tunnel1->npaths, (size_t)2);
+ KUNIT_ASSERT_EQ(test, tunnel1->npaths, 2);
KUNIT_ASSERT_EQ(test, tunnel1->paths[0]->path_length, 2);
KUNIT_EXPECT_PTR_EQ(test, tunnel1->paths[0]->hops[0].in_port, down);
KUNIT_EXPECT_PTR_EQ(test, tunnel1->paths[0]->hops[1].out_port, up);
@@ -1245,10 +1223,10 @@ static void tb_test_tunnel_pcie(struct kunit *test)
up = &dev2->ports[9];
tunnel2 = tb_tunnel_alloc_pci(NULL, up, down);
KUNIT_ASSERT_TRUE(test, tunnel2 != NULL);
- KUNIT_EXPECT_EQ(test, tunnel2->type, (enum tb_tunnel_type)TB_TUNNEL_PCI);
+ KUNIT_EXPECT_EQ(test, tunnel2->type, TB_TUNNEL_PCI);
KUNIT_EXPECT_PTR_EQ(test, tunnel2->src_port, down);
KUNIT_EXPECT_PTR_EQ(test, tunnel2->dst_port, up);
- KUNIT_ASSERT_EQ(test, tunnel2->npaths, (size_t)2);
+ KUNIT_ASSERT_EQ(test, tunnel2->npaths, 2);
KUNIT_ASSERT_EQ(test, tunnel2->paths[0]->path_length, 2);
KUNIT_EXPECT_PTR_EQ(test, tunnel2->paths[0]->hops[0].in_port, down);
KUNIT_EXPECT_PTR_EQ(test, tunnel2->paths[0]->hops[1].out_port, up);
@@ -1282,10 +1260,10 @@ static void tb_test_tunnel_dp(struct kunit *test)

tunnel = tb_tunnel_alloc_dp(NULL, in, out, 0, 0);
KUNIT_ASSERT_TRUE(test, tunnel != NULL);
- KUNIT_EXPECT_EQ(test, tunnel->type, (enum tb_tunnel_type)TB_TUNNEL_DP);
+ KUNIT_EXPECT_EQ(test, tunnel->type, TB_TUNNEL_DP);
KUNIT_EXPECT_PTR_EQ(test, tunnel->src_port, in);
KUNIT_EXPECT_PTR_EQ(test, tunnel->dst_port, out);
- KUNIT_ASSERT_EQ(test, tunnel->npaths, (size_t)3);
+ KUNIT_ASSERT_EQ(test, tunnel->npaths, 3);
KUNIT_ASSERT_EQ(test, tunnel->paths[0]->path_length, 2);
KUNIT_EXPECT_PTR_EQ(test, tunnel->paths[0]->hops[0].in_port, in);
KUNIT_EXPECT_PTR_EQ(test, tunnel->paths[0]->hops[1].out_port, out);
@@ -1328,10 +1306,10 @@ static void tb_test_tunnel_dp_chain(struct kunit *test)

tunnel = tb_tunnel_alloc_dp(NULL, in, out, 0, 0);
KUNIT_ASSERT_TRUE(test, tunnel != NULL);
- KUNIT_EXPECT_EQ(test, tunnel->type, (enum tb_tunnel_type)TB_TUNNEL_DP);
+ KUNIT_EXPECT_EQ(test, tunnel->type, TB_TUNNEL_DP);
KUNIT_EXPECT_PTR_EQ(test, tunnel->src_port, in);
KUNIT_EXPECT_PTR_EQ(test, tunnel->dst_port, out);
- KUNIT_ASSERT_EQ(test, tunnel->npaths, (size_t)3);
+ KUNIT_ASSERT_EQ(test, tunnel->npaths, 3);
KUNIT_ASSERT_EQ(test, tunnel->paths[0]->path_length, 3);
KUNIT_EXPECT_PTR_EQ(test, tunnel->paths[0]->hops[0].in_port, in);
KUNIT_EXPECT_PTR_EQ(test, tunnel->paths[0]->hops[2].out_port, out);
@@ -1378,10 +1356,10 @@ static void tb_test_tunnel_dp_tree(struct kunit *test)

tunnel = tb_tunnel_alloc_dp(NULL, in, out, 0, 0);
KUNIT_ASSERT_TRUE(test, tunnel != NULL);
- KUNIT_EXPECT_EQ(test, tunnel->type, (enum tb_tunnel_type)TB_TUNNEL_DP);
+ KUNIT_EXPECT_EQ(test, tunnel->type, TB_TUNNEL_DP);
KUNIT_EXPECT_PTR_EQ(test, tunnel->src_port, in);
KUNIT_EXPECT_PTR_EQ(test, tunnel->dst_port, out);
- KUNIT_ASSERT_EQ(test, tunnel->npaths, (size_t)3);
+ KUNIT_ASSERT_EQ(test, tunnel->npaths, 3);
KUNIT_ASSERT_EQ(test, tunnel->paths[0]->path_length, 4);
KUNIT_EXPECT_PTR_EQ(test, tunnel->paths[0]->hops[0].in_port, in);
KUNIT_EXPECT_PTR_EQ(test, tunnel->paths[0]->hops[3].out_port, out);
@@ -1443,10 +1421,10 @@ static void tb_test_tunnel_dp_max_length(struct kunit *test)

tunnel = tb_tunnel_alloc_dp(NULL, in, out, 0, 0);
KUNIT_ASSERT_TRUE(test, tunnel != NULL);
- KUNIT_EXPECT_EQ(test, tunnel->type, (enum tb_tunnel_type)TB_TUNNEL_DP);
+ KUNIT_EXPECT_EQ(test, tunnel->type, TB_TUNNEL_DP);
KUNIT_EXPECT_PTR_EQ(test, tunnel->src_port, in);
KUNIT_EXPECT_PTR_EQ(test, tunnel->dst_port, out);
- KUNIT_ASSERT_EQ(test, tunnel->npaths, (size_t)3);
+ KUNIT_ASSERT_EQ(test, tunnel->npaths, 3);
KUNIT_ASSERT_EQ(test, tunnel->paths[0]->path_length, 13);
/* First hop */
KUNIT_EXPECT_PTR_EQ(test, tunnel->paths[0]->hops[0].in_port, in);
@@ -1499,10 +1477,10 @@ static void tb_test_tunnel_usb3(struct kunit *test)
up = &dev1->ports[16];
tunnel1 = tb_tunnel_alloc_usb3(NULL, up, down, 0, 0);
KUNIT_ASSERT_TRUE(test, tunnel1 != NULL);
- KUNIT_EXPECT_EQ(test, tunnel1->type, (enum tb_tunnel_type)TB_TUNNEL_USB3);
+ KUNIT_EXPECT_EQ(test, tunnel1->type, TB_TUNNEL_USB3);
KUNIT_EXPECT_PTR_EQ(test, tunnel1->src_port, down);
KUNIT_EXPECT_PTR_EQ(test, tunnel1->dst_port, up);
- KUNIT_ASSERT_EQ(test, tunnel1->npaths, (size_t)2);
+ KUNIT_ASSERT_EQ(test, tunnel1->npaths, 2);
KUNIT_ASSERT_EQ(test, tunnel1->paths[0]->path_length, 2);
KUNIT_EXPECT_PTR_EQ(test, tunnel1->paths[0]->hops[0].in_port, down);
KUNIT_EXPECT_PTR_EQ(test, tunnel1->paths[0]->hops[1].out_port, up);
@@ -1514,10 +1492,10 @@ static void tb_test_tunnel_usb3(struct kunit *test)
up = &dev2->ports[16];
tunnel2 = tb_tunnel_alloc_usb3(NULL, up, down, 0, 0);
KUNIT_ASSERT_TRUE(test, tunnel2 != NULL);
- KUNIT_EXPECT_EQ(test, tunnel2->type, (enum tb_tunnel_type)TB_TUNNEL_USB3);
+ KUNIT_EXPECT_EQ(test, tunnel2->type, TB_TUNNEL_USB3);
KUNIT_EXPECT_PTR_EQ(test, tunnel2->src_port, down);
KUNIT_EXPECT_PTR_EQ(test, tunnel2->dst_port, up);
- KUNIT_ASSERT_EQ(test, tunnel2->npaths, (size_t)2);
+ KUNIT_ASSERT_EQ(test, tunnel2->npaths, 2);
KUNIT_ASSERT_EQ(test, tunnel2->paths[0]->path_length, 2);
KUNIT_EXPECT_PTR_EQ(test, tunnel2->paths[0]->hops[0].in_port, down);
KUNIT_EXPECT_PTR_EQ(test, tunnel2->paths[0]->hops[1].out_port, up);
@@ -1618,10 +1596,10 @@ static void tb_test_tunnel_dma(struct kunit *test)

tunnel = tb_tunnel_alloc_dma(NULL, nhi, port, 8, 1, 8, 1);
KUNIT_ASSERT_TRUE(test, tunnel != NULL);
- KUNIT_EXPECT_EQ(test, tunnel->type, (enum tb_tunnel_type)TB_TUNNEL_DMA);
+ KUNIT_EXPECT_EQ(test, tunnel->type, TB_TUNNEL_DMA);
KUNIT_EXPECT_PTR_EQ(test, tunnel->src_port, nhi);
KUNIT_EXPECT_PTR_EQ(test, tunnel->dst_port, port);
- KUNIT_ASSERT_EQ(test, tunnel->npaths, (size_t)2);
+ KUNIT_ASSERT_EQ(test, tunnel->npaths, 2);
/* RX path */
KUNIT_ASSERT_EQ(test, tunnel->paths[0]->path_length, 1);
KUNIT_EXPECT_PTR_EQ(test, tunnel->paths[0]->hops[0].in_port, port);
@@ -1661,10 +1639,10 @@ static void tb_test_tunnel_dma_rx(struct kunit *test)

tunnel = tb_tunnel_alloc_dma(NULL, nhi, port, -1, -1, 15, 2);
KUNIT_ASSERT_TRUE(test, tunnel != NULL);
- KUNIT_EXPECT_EQ(test, tunnel->type, (enum tb_tunnel_type)TB_TUNNEL_DMA);
+ KUNIT_EXPECT_EQ(test, tunnel->type, TB_TUNNEL_DMA);
KUNIT_EXPECT_PTR_EQ(test, tunnel->src_port, nhi);
KUNIT_EXPECT_PTR_EQ(test, tunnel->dst_port, port);
- KUNIT_ASSERT_EQ(test, tunnel->npaths, (size_t)1);
+ KUNIT_ASSERT_EQ(test, tunnel->npaths, 1);
/* RX path */
KUNIT_ASSERT_EQ(test, tunnel->paths[0]->path_length, 1);
KUNIT_EXPECT_PTR_EQ(test, tunnel->paths[0]->hops[0].in_port, port);
@@ -1698,10 +1676,10 @@ static void tb_test_tunnel_dma_tx(struct kunit *test)

tunnel = tb_tunnel_alloc_dma(NULL, nhi, port, 15, 2, -1, -1);
KUNIT_ASSERT_TRUE(test, tunnel != NULL);
- KUNIT_EXPECT_EQ(test, tunnel->type, (enum tb_tunnel_type)TB_TUNNEL_DMA);
+ KUNIT_EXPECT_EQ(test, tunnel->type, TB_TUNNEL_DMA);
KUNIT_EXPECT_PTR_EQ(test, tunnel->src_port, nhi);
KUNIT_EXPECT_PTR_EQ(test, tunnel->dst_port, port);
- KUNIT_ASSERT_EQ(test, tunnel->npaths, (size_t)1);
+ KUNIT_ASSERT_EQ(test, tunnel->npaths, 1);
/* TX path */
KUNIT_ASSERT_EQ(test, tunnel->paths[0]->path_length, 1);
KUNIT_EXPECT_PTR_EQ(test, tunnel->paths[0]->hops[0].in_port, nhi);
@@ -1744,10 +1722,10 @@ static void tb_test_tunnel_dma_chain(struct kunit *test)
port = &dev2->ports[3];
tunnel = tb_tunnel_alloc_dma(NULL, nhi, port, 8, 1, 8, 1);
KUNIT_ASSERT_TRUE(test, tunnel != NULL);
- KUNIT_EXPECT_EQ(test, tunnel->type, (enum tb_tunnel_type)TB_TUNNEL_DMA);
+ KUNIT_EXPECT_EQ(test, tunnel->type, TB_TUNNEL_DMA);
KUNIT_EXPECT_PTR_EQ(test, tunnel->src_port, nhi);
KUNIT_EXPECT_PTR_EQ(test, tunnel->dst_port, port);
- KUNIT_ASSERT_EQ(test, tunnel->npaths, (size_t)2);
+ KUNIT_ASSERT_EQ(test, tunnel->npaths, 2);
/* RX path */
KUNIT_ASSERT_EQ(test, tunnel->paths[0]->path_length, 3);
KUNIT_EXPECT_PTR_EQ(test, tunnel->paths[0]->hops[0].in_port, port);
@@ -1906,7 +1884,7 @@ static void tb_test_property_parse(struct kunit *test)

p = tb_property_find(dir, "vendorid", TB_PROPERTY_TYPE_VALUE);
KUNIT_ASSERT_TRUE(test, p != NULL);
- KUNIT_EXPECT_EQ(test, p->value.immediate, (u32)0xa27);
+ KUNIT_EXPECT_EQ(test, p->value.immediate, 0xa27);

p = tb_property_find(dir, "deviceid", TB_PROPERTY_TYPE_TEXT);
KUNIT_ASSERT_TRUE(test, p != NULL);
@@ -1914,7 +1892,7 @@ static void tb_test_property_parse(struct kunit *test)

p = tb_property_find(dir, "deviceid", TB_PROPERTY_TYPE_VALUE);
KUNIT_ASSERT_TRUE(test, p != NULL);
- KUNIT_EXPECT_EQ(test, p->value.immediate, (u32)0xa);
+ KUNIT_EXPECT_EQ(test, p->value.immediate, 0xa);

p = tb_property_find(dir, "missing", TB_PROPERTY_TYPE_DIRECTORY);
KUNIT_ASSERT_TRUE(test, !p);
@@ -1927,19 +1905,19 @@ static void tb_test_property_parse(struct kunit *test)

p = tb_property_find(network_dir, "prtcid", TB_PROPERTY_TYPE_VALUE);
KUNIT_ASSERT_TRUE(test, p != NULL);
- KUNIT_EXPECT_EQ(test, p->value.immediate, (u32)0x1);
+ KUNIT_EXPECT_EQ(test, p->value.immediate, 0x1);

p = tb_property_find(network_dir, "prtcvers", TB_PROPERTY_TYPE_VALUE);
KUNIT_ASSERT_TRUE(test, p != NULL);
- KUNIT_EXPECT_EQ(test, p->value.immediate, (u32)0x1);
+ KUNIT_EXPECT_EQ(test, p->value.immediate, 0x1);

p = tb_property_find(network_dir, "prtcrevs", TB_PROPERTY_TYPE_VALUE);
KUNIT_ASSERT_TRUE(test, p != NULL);
- KUNIT_EXPECT_EQ(test, p->value.immediate, (u32)0x1);
+ KUNIT_EXPECT_EQ(test, p->value.immediate, 0x1);

p = tb_property_find(network_dir, "prtcstns", TB_PROPERTY_TYPE_VALUE);
KUNIT_ASSERT_TRUE(test, p != NULL);
- KUNIT_EXPECT_EQ(test, p->value.immediate, (u32)0x0);
+ KUNIT_EXPECT_EQ(test, p->value.immediate, 0x0);

p = tb_property_find(network_dir, "deviceid", TB_PROPERTY_TYPE_VALUE);
KUNIT_EXPECT_TRUE(test, !p);
@@ -1960,7 +1938,7 @@ static void tb_test_property_format(struct kunit *test)
KUNIT_ASSERT_TRUE(test, dir != NULL);

ret = tb_property_format_dir(dir, NULL, 0);
- KUNIT_ASSERT_EQ(test, ret, (int)ARRAY_SIZE(root_directory));
+ KUNIT_ASSERT_EQ(test, ret, ARRAY_SIZE(root_directory));

block_len = ret;

@@ -2063,7 +2041,7 @@ static void tb_test_property_copy(struct kunit *test)

/* Compare the resulting property block */
ret = tb_property_format_dir(dst, NULL, 0);
- KUNIT_ASSERT_EQ(test, ret, (int)ARRAY_SIZE(root_directory));
+ KUNIT_ASSERT_EQ(test, ret, ARRAY_SIZE(root_directory));

block = kunit_kzalloc(test, sizeof(root_directory), GFP_KERNEL);
KUNIT_ASSERT_TRUE(test, block != NULL);
--
2.31.1.751.gd2f1c929bd-goog

[David Gow]

With some of the stricter type checking in KUnit's EXPECT macros

removed, several casts in sysctl-test are no longer required.

Remove the unnecessary casts, making the conditions clearer.

Signed-off-by: David Gow <davi...@google.com>
---
This should be a no-op functionality wise, and while it depends on the
first couple of patches in this series, it's otherwise independent from
the others. I think this makes the test more readable, but if you
particularly dislike it, I'm happy to drop it.

kernel/sysctl-test.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/kernel/sysctl-test.c b/kernel/sysctl-test.c
index ccb78509f1a8..664ded05dd7a 100644
--- a/kernel/sysctl-test.c
+++ b/kernel/sysctl-test.c
@@ -49,7 +49,7 @@ static void sysctl_test_api_dointvec_null_tbl_data(struct kunit *test)
KUNIT_EXPECT_EQ(test, 0, proc_dointvec(&null_data_table,
KUNIT_PROC_READ, buffer, &len,
&pos));
- KUNIT_EXPECT_EQ(test, (size_t)0, len);
+ KUNIT_EXPECT_EQ(test, 0, len);

/*
* See above.
@@ -58,7 +58,7 @@ static void sysctl_test_api_dointvec_null_tbl_data(struct kunit *test)
KUNIT_EXPECT_EQ(test, 0, proc_dointvec(&null_data_table,
KUNIT_PROC_WRITE, buffer, &len,
&pos));
- KUNIT_EXPECT_EQ(test, (size_t)0, len);
+ KUNIT_EXPECT_EQ(test, 0, len);
}

/*
@@ -95,7 +95,7 @@ static void sysctl_test_api_dointvec_table_maxlen_unset(struct kunit *test)
KUNIT_EXPECT_EQ(test, 0, proc_dointvec(&data_maxlen_unset_table,
KUNIT_PROC_READ, buffer, &len,
&pos));
- KUNIT_EXPECT_EQ(test, (size_t)0, len);
+ KUNIT_EXPECT_EQ(test, 0, len);

/*
* See previous comment.
@@ -104,7 +104,7 @@ static void sysctl_test_api_dointvec_table_maxlen_unset(struct kunit *test)
KUNIT_EXPECT_EQ(test, 0, proc_dointvec(&data_maxlen_unset_table,
KUNIT_PROC_WRITE, buffer, &len,
&pos));
- KUNIT_EXPECT_EQ(test, (size_t)0, len);
+ KUNIT_EXPECT_EQ(test, 0, len);
}

/*
@@ -135,11 +135,11 @@ static void sysctl_test_api_dointvec_table_len_is_zero(struct kunit *test)

KUNIT_EXPECT_EQ(test, 0, proc_dointvec(&table, KUNIT_PROC_READ, buffer,
&len, &pos));
- KUNIT_EXPECT_EQ(test, (size_t)0, len);
+ KUNIT_EXPECT_EQ(test, 0, len);

KUNIT_EXPECT_EQ(test, 0, proc_dointvec(&table, KUNIT_PROC_WRITE, buffer,
&len, &pos));
- KUNIT_EXPECT_EQ(test, (size_t)0, len);
+ KUNIT_EXPECT_EQ(test, 0, len);
}

/*
@@ -174,7 +174,7 @@ static void sysctl_test_api_dointvec_table_read_but_position_set(

KUNIT_EXPECT_EQ(test, 0, proc_dointvec(&table, KUNIT_PROC_READ, buffer,
&len, &pos));
- KUNIT_EXPECT_EQ(test, (size_t)0, len);
+ KUNIT_EXPECT_EQ(test, 0, len);
}

/*
@@ -203,7 +203,7 @@ static void sysctl_test_dointvec_read_happy_single_positive(struct kunit *test)

KUNIT_EXPECT_EQ(test, 0, proc_dointvec(&table, KUNIT_PROC_READ,
user_buffer, &len, &pos));
- KUNIT_ASSERT_EQ(test, (size_t)3, len);
+ KUNIT_ASSERT_EQ(test, 3, len);
buffer[len] = '\0';
/* And we read 13 back out. */
KUNIT_EXPECT_STREQ(test, "13\n", buffer);
@@ -233,9 +233,9 @@ static void sysctl_test_dointvec_read_happy_single_negative(struct kunit *test)

KUNIT_EXPECT_EQ(test, 0, proc_dointvec(&table, KUNIT_PROC_READ,
user_buffer, &len, &pos));
- KUNIT_ASSERT_EQ(test, (size_t)4, len);
+ KUNIT_ASSERT_EQ(test, 4, len);
buffer[len] = '\0';
- KUNIT_EXPECT_STREQ(test, "-16\n", (char *)buffer);
+ KUNIT_EXPECT_STREQ(test, "-16\n", buffer);
}

/*
@@ -265,7 +265,7 @@ static void sysctl_test_dointvec_write_happy_single_positive(struct kunit *test)
KUNIT_EXPECT_EQ(test, 0, proc_dointvec(&table, KUNIT_PROC_WRITE,
user_buffer, &len, &pos));
KUNIT_EXPECT_EQ(test, sizeof(input) - 1, len);
- KUNIT_EXPECT_EQ(test, sizeof(input) - 1, (size_t)pos);
+ KUNIT_EXPECT_EQ(test, sizeof(input) - 1, pos);
KUNIT_EXPECT_EQ(test, 9, *((int *)table.data));
}

@@ -295,7 +295,7 @@ static void sysctl_test_dointvec_write_happy_single_negative(struct kunit *test)
KUNIT_EXPECT_EQ(test, 0, proc_dointvec(&table, KUNIT_PROC_WRITE,
user_buffer, &len, &pos));
KUNIT_EXPECT_EQ(test, sizeof(input) - 1, len);
- KUNIT_EXPECT_EQ(test, sizeof(input) - 1, (size_t)pos);
+ KUNIT_EXPECT_EQ(test, sizeof(input) - 1, pos);
KUNIT_EXPECT_EQ(test, -9, *((int *)table.data));
}

--
2.31.1.751.gd2f1c929bd-goog

[David Gow]

With some of the stricter type checking in KUnit's EXPECT macros

removed, several casts in policy_unpack_test are no longer required.

Remove the unnecessary casts, making the conditions clearer.

Signed-off-by: David Gow <davi...@google.com>
---
This should be a no-op functionality wise, and while it depends on the
first couple of patches in this series, it's otherwise independent from
the others. I think this makes the test more readable, but if you
particularly dislike it, I'm happy to drop it.

security/apparmor/policy_unpack_test.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/security/apparmor/policy_unpack_test.c b/security/apparmor/policy_unpack_test.c
index 533137f45361..03f78a41ef79 100644
--- a/security/apparmor/policy_unpack_test.c
+++ b/security/apparmor/policy_unpack_test.c
@@ -177,7 +177,7 @@ static void policy_unpack_test_unpack_array_out_of_bounds(struct kunit *test)

array_size = unpack_array(puf->e, name);

- KUNIT_EXPECT_EQ(test, array_size, (u16)0);
+ KUNIT_EXPECT_EQ(test, array_size, 0);
KUNIT_EXPECT_PTR_EQ(test, puf->e->pos,
puf->e->start + TEST_NAMED_ARRAY_BUF_OFFSET);
}
@@ -313,7 +313,7 @@ static void policy_unpack_test_unpack_strdup_out_of_bounds(struct kunit *test)
size = unpack_strdup(puf->e, &string, TEST_STRING_NAME);

KUNIT_EXPECT_EQ(test, size, 0);
- KUNIT_EXPECT_PTR_EQ(test, string, (char *)NULL);
+ KUNIT_EXPECT_PTR_EQ(test, string, NULL);
KUNIT_EXPECT_PTR_EQ(test, puf->e->pos, start);
}

@@ -391,10 +391,10 @@ static void policy_unpack_test_unpack_u16_chunk_basic(struct kunit *test)

size = unpack_u16_chunk(puf->e, &chunk);

- KUNIT_EXPECT_PTR_EQ(test, (void *)chunk,
+ KUNIT_EXPECT_PTR_EQ(test, chunk,
puf->e->start + TEST_U16_OFFSET + 2);
- KUNIT_EXPECT_EQ(test, size, (size_t)TEST_U16_DATA);
- KUNIT_EXPECT_PTR_EQ(test, puf->e->pos, (void *)(chunk + TEST_U16_DATA));
+ KUNIT_EXPECT_EQ(test, size, TEST_U16_DATA);
+ KUNIT_EXPECT_PTR_EQ(test, puf->e->pos, (chunk + TEST_U16_DATA));
}

static void policy_unpack_test_unpack_u16_chunk_out_of_bounds_1(
@@ -408,8 +408,8 @@ static void policy_unpack_test_unpack_u16_chunk_out_of_bounds_1(

size = unpack_u16_chunk(puf->e, &chunk);

- KUNIT_EXPECT_EQ(test, size, (size_t)0);
- KUNIT_EXPECT_PTR_EQ(test, chunk, (char *)NULL);
+ KUNIT_EXPECT_EQ(test, size, 0);
+ KUNIT_EXPECT_PTR_EQ(test, chunk, NULL);
KUNIT_EXPECT_PTR_EQ(test, puf->e->pos, puf->e->end - 1);
}

@@ -430,8 +430,8 @@ static void policy_unpack_test_unpack_u16_chunk_out_of_bounds_2(

size = unpack_u16_chunk(puf->e, &chunk);

- KUNIT_EXPECT_EQ(test, size, (size_t)0);
- KUNIT_EXPECT_PTR_EQ(test, chunk, (char *)NULL);
+ KUNIT_EXPECT_EQ(test, size, 0);
+ KUNIT_EXPECT_PTR_EQ(test, chunk, NULL);
KUNIT_EXPECT_PTR_EQ(test, puf->e->pos, puf->e->start + TEST_U16_OFFSET);
}

--
2.31.1.751.gd2f1c929bd-goog

[David Gow]

With some of the stricter type checking in KUnit's EXPECT macros

removed, a cast in cmdline_kunit is no longer required.

Remove the unnecessary cast, using NULL instead of (int *) to make it

clearer.

Signed-off-by: David Gow <davi...@google.com>
---
This should be a no-op functionality wise, and while it depends on the
first couple of patches in this series, it's otherwise independent from
the others. I think this makes the test more readable, but if you
particularly dislike it, I'm happy to drop it.

lib/cmdline_kunit.c | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/cmdline_kunit.c b/lib/cmdline_kunit.c
index 018bfc8113c4..a72a2c16066e 100644
--- a/lib/cmdline_kunit.c
+++ b/lib/cmdline_kunit.c
@@ -124,7 +124,7 @@ static void cmdline_do_one_range_test(struct kunit *test, const char *in,
n, e[0], r[0]);

p = memchr_inv(&r[1], 0, sizeof(r) - sizeof(r[0]));
- KUNIT_EXPECT_PTR_EQ_MSG(test, p, (int *)0, "in test %u at %u out of bound", n, p - r);
+ KUNIT_EXPECT_PTR_EQ_MSG(test, p, NULL, "in test %u at %u out of bound", n, p - r);
}

static void cmdline_test_range(struct kunit *test)
--
2.31.1.751.gd2f1c929bd-goog

[Greg Kroah-Hartman]

On Thu, May 13, 2021 at 12:31:58PM -0700, David Gow wrote:
> With some of the stricter type checking in KUnit's EXPECT macros
> removed, several casts in property-entry-test are no longer required.
>
> Remove the unnecessary casts, making the conditions clearer.
>
> Signed-off-by: David Gow <davi...@google.com>

Acked-by: Greg Kroah-Hartman <gre...@linuxfoundation.org>

[Daniel Latypov]

On Thu, May 13, 2021 at 12:36 PM David Gow <davi...@google.com> wrote:
>
> As the type checking is no longer excessively strict, get rid of the
> unsightly (char*) casts -- and comment discussing them -- from the KUnit
> usage page.
>
> Signed-off-by: David Gow <davi...@google.com>

Reviewed-by: Daniel Latypov <dlat...@google.com>

Thanks!

[Andrew Jeffery]

On Fri, 14 May 2021, at 05:02, David Gow wrote:
> With KUnit's EXPECT macros no longer typechecking arguments as strictly,
> get rid of a number of now unnecessary casts.
>
> Signed-off-by: David Gow <davi...@google.com>
> ---
> This should be a no-op functionality wise, and while it depends on the
> first couple of patches in this series, it's otherwise independent from
> the others. I think this makes the test more readable, but if you
> particularly dislike it, I'm happy to drop it.

No, happy to have that cleaned up.

Thanks David.

Acked-by: Andrew Jeffery <and...@aj.id.au>

[Mika Westerberg]

Hi,

On Thu, May 13, 2021 at 12:32:01PM -0700, David Gow wrote:
> With some of the stricter type checking in KUnit's EXPECT macros
> removed, several casts in the thunderbolt KUnit tests are no longer
> required.
>
> Remove the unnecessary casts, making the conditions clearer.
>
> Signed-off-by: David Gow <davi...@google.com>

Looks good.

Does this go through KUnit tree or you want me to take it? In case of
the former feel free to add:

Acked-by: Mika Westerberg <mika.we...@linux.intel.com>

[David Gow]

Thanks. I think it's probably easier for this to go in via the KUnit
tree, unless Brendan or Shuah have any objections.

Cheers,
-- David

[Shuah Khan]

It is fine either way unless there are dependencies on the KUnit
tree. I can take this in through KUnit once Brendan looks at it.

thanks,
-- Shuah

[Andy Shevchenko]

On Fri, May 14, 2021 at 2:32 AM David Gow <davi...@google.com> wrote:
>
> With some of the stricter type checking in KUnit's EXPECT macros
> removed, a cast in cmdline_kunit is no longer required.
>
> Remove the unnecessary cast, using NULL instead of (int *) to make it
> clearer.

Acked-by: Andy Shevchenko <andy.sh...@gmail.com>

> Signed-off-by: David Gow <davi...@google.com>
> ---
> This should be a no-op functionality wise, and while it depends on the
> first couple of patches in this series, it's otherwise independent from
> the others. I think this makes the test more readable, but if you
> particularly dislike it, I'm happy to drop it.
>
> lib/cmdline_kunit.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/cmdline_kunit.c b/lib/cmdline_kunit.c
> index 018bfc8113c4..a72a2c16066e 100644
> --- a/lib/cmdline_kunit.c
> +++ b/lib/cmdline_kunit.c
> @@ -124,7 +124,7 @@ static void cmdline_do_one_range_test(struct kunit *test, const char *in,
> n, e[0], r[0]);
>
> p = memchr_inv(&r[1], 0, sizeof(r) - sizeof(r[0]));
> - KUNIT_EXPECT_PTR_EQ_MSG(test, p, (int *)0, "in test %u at %u out of bound", n, p - r);
> + KUNIT_EXPECT_PTR_EQ_MSG(test, p, NULL, "in test %u at %u out of bound", n, p - r);
> }
>
> static void cmdline_test_range(struct kunit *test)
> --
> 2.31.1.751.gd2f1c929bd-goog
>

--
With Best Regards,
Andy Shevchenko

[Ulf Hansson]

On Thu, 13 May 2021 at 21:36, David Gow <davi...@google.com> wrote:
>
> With KUnit's EXPECT macros no longer typechecking arguments as strictly,
> get rid of a number of now unnecessary casts.
>
> Signed-off-by: David Gow <davi...@google.com>

I guess you will funnel this via another tree than the mmc?

Acked-by: Ulf Hansson <ulf.h...@linaro.org>

Kind regards
Uffe

[David Gow]

On Mon, May 17, 2021 at 5:23 PM Ulf Hansson <ulf.h...@linaro.org> wrote:
>
> On Thu, 13 May 2021 at 21:36, David Gow <davi...@google.com> wrote:
> >
> > With KUnit's EXPECT macros no longer typechecking arguments as strictly,
> > get rid of a number of now unnecessary casts.
> >
> > Signed-off-by: David Gow <davi...@google.com>
>
> I guess you will funnel this via another tree than the mmc?
>
> Acked-by: Ulf Hansson <ulf.h...@linaro.org>
>
> Kind regards
> Uffe
>

Yeah: the plan is to have this whole series go through the
kselftest/kunit tree so we don't have to worry about potentially
temporarily introducing a bunch of compiler warnings.

Cheers,
-- David

[Brendan Higgins]

On Thu, May 13, 2021 at 12:36 PM David Gow <davi...@google.com> wrote:
>

> As the type checking is no longer excessively strict, get rid of the
> unsightly (char*) casts -- and comment discussing them -- from the KUnit
> usage page.
>
> Signed-off-by: David Gow <davi...@google.com>

Acked-by: Brendan Higgins <brendan...@google.com>

[Brendan Higgins]

On Thu, May 13, 2021 at 12:36 PM David Gow <davi...@google.com> wrote:
>

> With some of the stricter type checking in KUnit's EXPECT macros
> removed, several casts in property-entry-test are no longer required.
>
> Remove the unnecessary casts, making the conditions clearer.
>
> Signed-off-by: David Gow <davi...@google.com>

Reviewed-by: Brendan Higgins <brendan...@google.com>

[Brendan Higgins]

On Thu, May 13, 2021 at 12:36 PM David Gow <davi...@google.com> wrote:
>

> KUnit's EXPECT macros no longer typecheck as stringently, so casting the
> result of strcmp() is now unnecessary.
>
> Signed-off-by: David Gow <davi...@google.com>

Reviewed-by: Brendan Higgins <brendan...@google.com>

[Brendan Higgins]

On Thu, May 13, 2021 at 12:36 PM David Gow <davi...@google.com> wrote:
>

> With KUnit's EXPECT macros no longer typechecking arguments as strictly,
> get rid of a number of now unnecessary casts.
>
> Signed-off-by: David Gow <davi...@google.com>

Reviewed-by: Brendan Higgins <brendan...@google.com>

[Brendan Higgins]

On Thu, May 13, 2021 at 12:36 PM David Gow <davi...@google.com> wrote:
>

> With some of the stricter type checking in KUnit's EXPECT macros
> removed, several casts in the thunderbolt KUnit tests are no longer
> required.
>
> Remove the unnecessary casts, making the conditions clearer.
>
> Signed-off-by: David Gow <davi...@google.com>

Reviewed-by: Brendan Higgins <brendan...@google.com>

[Brendan Higgins]

On Thu, May 13, 2021 at 12:36 PM David Gow <davi...@google.com> wrote:
>

> With some of the stricter type checking in KUnit's EXPECT macros
> removed, several casts in sysctl-test are no longer required.
>
> Remove the unnecessary casts, making the conditions clearer.
>
> Signed-off-by: David Gow <davi...@google.com>

Reviewed-by: Brendan Higgins <brendan...@google.com>

[Brendan Higgins]

On Thu, May 13, 2021 at 12:36 PM David Gow <davi...@google.com> wrote:
>

> With some of the stricter type checking in KUnit's EXPECT macros
> removed, several casts in policy_unpack_test are no longer required.
>
> Remove the unnecessary casts, making the conditions clearer.
>
> Signed-off-by: David Gow <davi...@google.com>

Reviewed-by: Brendan Higgins <brendan...@google.com>

[Brendan Higgins]

On Thu, May 13, 2021 at 12:36 PM David Gow <davi...@google.com> wrote:
>

> With some of the stricter type checking in KUnit's EXPECT macros
> removed, a cast in cmdline_kunit is no longer required.
>
> Remove the unnecessary cast, using NULL instead of (int *) to make it
> clearer.
>
> Signed-off-by: David Gow <davi...@google.com>

Reviewed-by: Brendan Higgins <brendan...@google.com>

[John Johansen]

On 5/13/21 12:32 PM, David Gow wrote:
> With some of the stricter type checking in KUnit's EXPECT macros
> removed, several casts in policy_unpack_test are no longer required.
>
> Remove the unnecessary casts, making the conditions clearer.
>
> Signed-off-by: David Gow <davi...@google.com>

Acked-by: John Johansen <john.j...@canonical.com>

I have pulled this into the apparmor tree

[Jonathan Cameron]

Seems sensible

Acked-by: Jonathan Cameron <Jonathan...@huawei.com>