Linux-kselftest Archive on lore.kernel.org
 help / color / Atom feed
* Re: What is the best way to compare an unsigned and a constant?
  2019-12-27 12:39 What is the best way to compare an unsigned and a constant? SeongJae Park
@ 2019-12-27 12:52 ` Bernd Petrovitsch
  2020-01-07 11:52   ` SeongJae Park
  0 siblings, 1 reply; 7+ messages in thread
From: Bernd Petrovitsch @ 2019-12-27 12:52 UTC (permalink / raw)
  To: SeongJae Park
  Cc: brendanhiggins, linux-kselftest, kunit-dev, linux-kernel, SeongJae Park

[-- Attachment #1: Type: text/plain, Size: 778 bytes --]

Hi all!

On 27/12/2019 13:39, SeongJae Park wrote:
[...]
> I have a function returning 'unsigned long', and would like to write a kunit
> test for the function, as below.
> 
>     unsigned long foo(void)
>     {
>     	return 42;
>     }
> 
>     static void foo_test(struct kunit *test)
>     {
>         KUNIT_EXPECT_EQ(test, 42, foo());
>     }

For this case: shouldn't 
----  snip  ----
static void foo_test(struct kunit *test)
{
     KUNIT_EXPECT_EQ(test, 42ul, foo());
}
----  snip  ----
do the trick?

MfG,
	Bernd
-- 
"I dislike type abstraction if it has no real reason. And saving
on typing is not a good reason - if your typing speed is the main
issue when you're coding, you're doing something seriously wrong."
    - Linus Torvalds

[-- Attachment #2: pEpkey.asc --]
[-- Type: application/pgp-keys, Size: 2513 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Re: What is the best way to compare an unsigned and a constant?
  2019-12-27 12:52 ` Bernd Petrovitsch
@ 2020-01-07 11:52   ` SeongJae Park
  2020-01-07 13:35     ` Brendan Higgins
  0 siblings, 1 reply; 7+ messages in thread
From: SeongJae Park @ 2020-01-07 11:52 UTC (permalink / raw)
  To: Bernd Petrovitsch
  Cc: SeongJae Park, brendanhiggins, linux-kselftest, kunit-dev,
	linux-kernel, SeongJae Park

On   Fri, 27 Dec 2019 13:52:27 +0100   Bernd Petrovitsch <bernd@petrovitsch.priv.at> wrote:

> This is a multi-part message in MIME format.
> --------------D98A0A31D62B0BC2939BAEE9
> Content-Type: text/plain; charset=utf-8
> Content-Transfer-Encoding: quoted-printable
> 
> Hi all!
> 
> On 27/12/2019 13:39, SeongJae Park wrote:
> [...]
> > I have a function returning 'unsigned long', and would like to write a =
> kunit
> > test for the function, as below.
> >=20
> >     unsigned long foo(void)
> >     {
> >     	return 42;
> >     }
> >=20
> >     static void foo_test(struct kunit *test)
> >     {
> >         KUNIT_EXPECT_EQ(test, 42, foo());
> >     }
> 
> For this case: shouldn't=20
> ----  snip  ----
> static void foo_test(struct kunit *test)
> {
>      KUNIT_EXPECT_EQ(test, 42ul, foo());
> }
> ----  snip  ----
> do the trick?

Unfortunately, it doesn't works.

    [13:04:58] Building KUnit Kernel ...
    In file included from /.../linux/include/linux/list.h:9:0,
                     from /.../linux/include/linux/wait.h:7,
                     from /.../linux/include/linux/wait_bit.h:8,
                     from /.../linux/include/linux/fs.h:6,
                     from /.../linux/include/linux/debugfs.h:15,
                     from /.../linux/mm/damon.c:12:
    /.../linux/mm/damon-test.h: In function ‘damon_test_foo’:
    /.../linux/include/linux/kernel.h:842:29: warning: comparison of distinct pointer types lacks a cast
       (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
                                 ^
    /.../linux/include/kunit/test.h:493:9: note: in expansion of macro ‘__typecheck’
      ((void)__typecheck(__left, __right));           \
             ^~~~~~~~~~~
    /.../linux/include/kunit/test.h:517:2: note: in expansion of macro ‘KUNIT_BASE_BINARY_ASSERTION’
      KUNIT_BASE_BINARY_ASSERTION(test,           \
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~
    /.../linux/include/kunit/test.h:606:2: note: in expansion of macro ‘KUNIT_BASE_EQ_MSG_ASSERTION’
      KUNIT_BASE_EQ_MSG_ASSERTION(test,           \
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~
    /.../linux/include/kunit/test.h:616:2: note: in expansion of macro ‘KUNIT_BINARY_EQ_MSG_ASSERTION’
      KUNIT_BINARY_EQ_MSG_ASSERTION(test,           \
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    /.../linux/include/kunit/test.h:979:2: note: in expansion of macro ‘KUNIT_BINARY_EQ_ASSERTION’
      KUNIT_BINARY_EQ_ASSERTION(test, KUNIT_EXPECTATION, left, right)
      ^~~~~~~~~~~~~~~~~~~~~~~~~
    /.../linux/mm/damon-test.h:565:2: note: in expansion of macro ‘KUNIT_EXPECT_EQ’
      KUNIT_EXPECT_EQ(test, 42ul, (int)foo());
      ^~~~~~~~~~~~~~~

Some other thoughts?


Thanks,
SeongJae Park


> 
> MfG,
> 	Bernd
> --=20
> "I dislike type abstraction if it has no real reason. And saving
> on typing is not a good reason - if your typing speed is the main
> issue when you're coding, you're doing something seriously wrong."
>     - Linus Torvalds

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Re: What is the best way to compare an unsigned and a constant?
  2020-01-07 11:52   ` SeongJae Park
@ 2020-01-07 13:35     ` Brendan Higgins
  2020-01-07 13:49       ` SeongJae Park
  0 siblings, 1 reply; 7+ messages in thread
From: Brendan Higgins @ 2020-01-07 13:35 UTC (permalink / raw)
  To: SeongJae Park
  Cc: Bernd Petrovitsch, open list:KERNEL SELFTEST FRAMEWORK,
	KUnit Development, Linux Kernel Mailing List, SeongJae Park

Sorry for the delay, I was on vacation. (I still am, but I was too ;-).)

On Tue, Jan 7, 2020 at 3:52 AM SeongJae Park <sjpark@amazon.com> wrote:
>
> On   Fri, 27 Dec 2019 13:52:27 +0100   Bernd Petrovitsch <bernd@petrovitsch.priv.at> wrote:
>
> > This is a multi-part message in MIME format.
> > --------------D98A0A31D62B0BC2939BAEE9
> > Content-Type: text/plain; charset=utf-8
> > Content-Transfer-Encoding: quoted-printable
> >
> > Hi all!
> >
> > On 27/12/2019 13:39, SeongJae Park wrote:
> > [...]
> > > I have a function returning 'unsigned long', and would like to write a =
> > kunit
> > > test for the function, as below.
> > >=20
> > >     unsigned long foo(void)
> > >     {
> > >             return 42;
> > >     }
> > >=20
> > >     static void foo_test(struct kunit *test)
> > >     {
> > >         KUNIT_EXPECT_EQ(test, 42, foo());
> > >     }
> >
> > For this case: shouldn't=20
> > ----  snip  ----
> > static void foo_test(struct kunit *test)
> > {
> >      KUNIT_EXPECT_EQ(test, 42ul, foo());
> > }
> > ----  snip  ----
> > do the trick?
>
> Unfortunately, it doesn't works.
>
>     [13:04:58] Building KUnit Kernel ...
>     In file included from /.../linux/include/linux/list.h:9:0,
>                      from /.../linux/include/linux/wait.h:7,
>                      from /.../linux/include/linux/wait_bit.h:8,
>                      from /.../linux/include/linux/fs.h:6,
>                      from /.../linux/include/linux/debugfs.h:15,
>                      from /.../linux/mm/damon.c:12:
>     /.../linux/mm/damon-test.h: In function ‘damon_test_foo’:
>     /.../linux/include/linux/kernel.h:842:29: warning: comparison of distinct pointer types lacks a cast
>        (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
>                                  ^
>     /.../linux/include/kunit/test.h:493:9: note: in expansion of macro ‘__typecheck’
>       ((void)__typecheck(__left, __right));           \
>              ^~~~~~~~~~~
>     /.../linux/include/kunit/test.h:517:2: note: in expansion of macro ‘KUNIT_BASE_BINARY_ASSERTION’
>       KUNIT_BASE_BINARY_ASSERTION(test,           \
>       ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>     /.../linux/include/kunit/test.h:606:2: note: in expansion of macro ‘KUNIT_BASE_EQ_MSG_ASSERTION’
>       KUNIT_BASE_EQ_MSG_ASSERTION(test,           \
>       ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>     /.../linux/include/kunit/test.h:616:2: note: in expansion of macro ‘KUNIT_BINARY_EQ_MSG_ASSERTION’
>       KUNIT_BINARY_EQ_MSG_ASSERTION(test,           \
>       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>     /.../linux/include/kunit/test.h:979:2: note: in expansion of macro ‘KUNIT_BINARY_EQ_ASSERTION’
>       KUNIT_BINARY_EQ_ASSERTION(test, KUNIT_EXPECTATION, left, right)
>       ^~~~~~~~~~~~~~~~~~~~~~~~~
>     /.../linux/mm/damon-test.h:565:2: note: in expansion of macro ‘KUNIT_EXPECT_EQ’
>       KUNIT_EXPECT_EQ(test, 42ul, (int)foo());
>       ^~~~~~~~~~~~~~~

Isn't the issue here that you fixed the 42, but are now casting the
result of foo() to an int?

Or have you fixed that now too?

Worst case (gross) scenario, you could just cast 42 to whatever type
foo() returns.

> Some other thoughts?
>
>
> Thanks,
> SeongJae Park
>
>
> >
> > MfG,
> >       Bernd
> > --=20
> > "I dislike type abstraction if it has no real reason. And saving
> > on typing is not a good reason - if your typing speed is the main
> > issue when you're coding, you're doing something seriously wrong."
> >     - Linus Torvalds

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Re: Re: What is the best way to compare an unsigned and a constant?
  2020-01-07 13:35     ` Brendan Higgins
@ 2020-01-07 13:49       ` SeongJae Park
  2020-01-08 14:12         ` Brendan Higgins
  0 siblings, 1 reply; 7+ messages in thread
From: SeongJae Park @ 2020-01-07 13:49 UTC (permalink / raw)
  To: Brendan Higgins
  Cc: SeongJae Park, Bernd Petrovitsch,
	open list:KERNEL SELFTEST FRAMEWORK, KUnit Development,
	Linux Kernel Mailing List, SeongJae Park

On   Tue, 7 Jan 2020 05:35:21 -0800   Brendan Higgins <brendanhiggins@google.com> wrote:

> Sorry for the delay, I was on vacation. (I still am, but I was too ;-).)

Happy new year, Brendan :)

> 
> On Tue, Jan 7, 2020 at 3:52 AM SeongJae Park <sjpark@amazon.com> wrote:
> >
> > On   Fri, 27 Dec 2019 13:52:27 +0100   Bernd Petrovitsch <bernd@petrovits=
> ch.priv.at> wrote:
> >
> > > This is a multi-part message in MIME format.
> > > --------------D98A0A31D62B0BC2939BAEE9
> > > Content-Type: text/plain; charset=3Dutf-8
> > > Content-Transfer-Encoding: quoted-printable
> > >
> > > Hi all!
> > >
> > > On 27/12/2019 13:39, SeongJae Park wrote:
> > > [...]
> > > > I have a function returning 'unsigned long', and would like to write =
> a =3D
> > > kunit
> > > > test for the function, as below.
> > > >=3D20
> > > >     unsigned long foo(void)
> > > >     {
> > > >             return 42;
> > > >     }
> > > >=3D20
> > > >     static void foo_test(struct kunit *test)
> > > >     {
> > > >         KUNIT_EXPECT_EQ(test, 42, foo());
> > > >     }
> > >
> > > For this case: shouldn't=3D20
> > > ----  snip  ----
> > > static void foo_test(struct kunit *test)
> > > {
> > >      KUNIT_EXPECT_EQ(test, 42ul, foo());
> > > }
> > > ----  snip  ----
> > > do the trick?
> >
> > Unfortunately, it doesn't works.
> >
> >     [13:04:58] Building KUnit Kernel ...
> >     In file included from /.../linux/include/linux/list.h:9:0,
> >                      from /.../linux/include/linux/wait.h:7,
> >                      from /.../linux/include/linux/wait_bit.h:8,
> >                      from /.../linux/include/linux/fs.h:6,
> >                      from /.../linux/include/linux/debugfs.h:15,
> >                      from /.../linux/mm/damon.c:12:
> >     /.../linux/mm/damon-test.h: In function =E2=80=98damon_test_foo=E2=80=
> =99:
> >     /.../linux/include/linux/kernel.h:842:29: warning: comparison of dist=
> inct pointer types lacks a cast
> >        (!!(sizeof((typeof(x) *)1 =3D=3D (typeof(y) *)1)))
> >                                  ^
> >     /.../linux/include/kunit/test.h:493:9: note: in expansion of macro =
> =E2=80=98__typecheck=E2=80=99
> >       ((void)__typecheck(__left, __right));           \
> >              ^~~~~~~~~~~
> >     /.../linux/include/kunit/test.h:517:2: note: in expansion of macro =
> =E2=80=98KUNIT_BASE_BINARY_ASSERTION=E2=80=99
> >       KUNIT_BASE_BINARY_ASSERTION(test,           \
> >       ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> >     /.../linux/include/kunit/test.h:606:2: note: in expansion of macro =
> =E2=80=98KUNIT_BASE_EQ_MSG_ASSERTION=E2=80=99
> >       KUNIT_BASE_EQ_MSG_ASSERTION(test,           \
> >       ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> >     /.../linux/include/kunit/test.h:616:2: note: in expansion of macro =
> =E2=80=98KUNIT_BINARY_EQ_MSG_ASSERTION=E2=80=99
> >       KUNIT_BINARY_EQ_MSG_ASSERTION(test,           \
> >       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >     /.../linux/include/kunit/test.h:979:2: note: in expansion of macro =
> =E2=80=98KUNIT_BINARY_EQ_ASSERTION=E2=80=99
> >       KUNIT_BINARY_EQ_ASSERTION(test, KUNIT_EXPECTATION, left, right)
> >       ^~~~~~~~~~~~~~~~~~~~~~~~~
> >     /.../linux/mm/damon-test.h:565:2: note: in expansion of macro =E2=80=
> =98KUNIT_EXPECT_EQ=E2=80=99
> >       KUNIT_EXPECT_EQ(test, 42ul, (int)foo());
> >       ^~~~~~~~~~~~~~~
> 
> Isn't the issue here that you fixed the 42, but are now casting the
> result of foo() to an int?

Oh, right...  Removing the non-sense casting fixed the problem.  Thanks,
Brendan!


Thanks,
SeongJae Park

> 
> Or have you fixed that now too?
> 
> Worst case (gross) scenario, you could just cast 42 to whatever type
> foo() returns.
> 
> > Some other thoughts?
> >
> >
> > Thanks,
> > SeongJae Park
> >
> >
> > >
> > > MfG,
> > >       Bernd
> > > --=3D20
> > > "I dislike type abstraction if it has no real reason. And saving
> > > on typing is not a good reason - if your typing speed is the main
> > > issue when you're coding, you're doing something seriously wrong."
> > >     - Linus Torvalds
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Re: Re: What is the best way to compare an unsigned and a constant?
  2020-01-07 13:49       ` SeongJae Park
@ 2020-01-08 14:12         ` Brendan Higgins
  2020-01-08 14:26           ` SeongJae Park
  0 siblings, 1 reply; 7+ messages in thread
From: Brendan Higgins @ 2020-01-08 14:12 UTC (permalink / raw)
  To: SeongJae Park
  Cc: Bernd Petrovitsch, open list:KERNEL SELFTEST FRAMEWORK,
	KUnit Development, Linux Kernel Mailing List, SeongJae Park

On Tue, Jan 7, 2020 at 5:49 AM SeongJae Park <sjpark@amazon.com> wrote:
>
> On   Tue, 7 Jan 2020 05:35:21 -0800   Brendan Higgins <brendanhiggins@google.com> wrote:
>
> > Sorry for the delay, I was on vacation. (I still am, but I was too ;-).)
>
> Happy new year, Brendan :)

Happy New Year!

> >
> > On Tue, Jan 7, 2020 at 3:52 AM SeongJae Park <sjpark@amazon.com> wrote:
> > >
> > > On   Fri, 27 Dec 2019 13:52:27 +0100   Bernd Petrovitsch <bernd@petrovits=
> > ch.priv.at> wrote:

[...]

> Oh, right...  Removing the non-sense casting fixed the problem.  Thanks,
> Brendan!

No worries, I do that kind of stuff all the time :-)

Does that fix everything? It looks like there was an encoding issue
with your last email, so I wasn't sure if I got everything.

Cheers!

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Re: Re: Re: What is the best way to compare an unsigned and a constant?
  2020-01-08 14:12         ` Brendan Higgins
@ 2020-01-08 14:26           ` SeongJae Park
  0 siblings, 0 replies; 7+ messages in thread
From: SeongJae Park @ 2020-01-08 14:26 UTC (permalink / raw)
  To: Brendan Higgins
  Cc: SeongJae Park, Bernd Petrovitsch,
	open list:KERNEL SELFTEST FRAMEWORK, KUnit Development,
	Linux Kernel Mailing List, SeongJae Park

On   Wed, 8 Jan 2020 06:12:47 -0800   Brendan Higgins <brendanhiggins@google.com> wrote:

> On Tue, Jan 7, 2020 at 5:49 AM SeongJae Park <sjpark@amazon.com> wrote:
> >
> > On   Tue, 7 Jan 2020 05:35:21 -0800   Brendan Higgins <brendanhiggins@google.com> wrote:
> >
> > > Sorry for the delay, I was on vacation. (I still am, but I was too ;-).)
> >
> > Happy new year, Brendan :)
> 
> Happy New Year!
> 
> > >
> > > On Tue, Jan 7, 2020 at 3:52 AM SeongJae Park <sjpark@amazon.com> wrote:
> > > >
> > > > On   Fri, 27 Dec 2019 13:52:27 +0100   Bernd Petrovitsch <bernd@petrovits=
> > > ch.priv.at> wrote:
> 
> [...]
> 
> > Oh, right...  Removing the non-sense casting fixed the problem.  Thanks,
> > Brendan!
> 
> No worries, I do that kind of stuff all the time :-)

Thanks :)

> 
> Does that fix everything? It looks like there was an encoding issue
> with your last email, so I wasn't sure if I got everything.

Yes, it fixed my every problem.  Both 'kunit' and 'checkpatch.pl' shows no
warning, now.


Thanks,
SeongJae Park

> 
> Cheers!
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* What is the best way to compare an unsigned and a constant?
@ 2019-12-27 12:39 SeongJae Park
  2019-12-27 12:52 ` Bernd Petrovitsch
  0 siblings, 1 reply; 7+ messages in thread
From: SeongJae Park @ 2019-12-27 12:39 UTC (permalink / raw)
  To: brendanhiggins; +Cc: linux-kselftest, kunit-dev, linux-kernel, SeongJae Park

Hello,


I have a function returning 'unsigned long', and would like to write a kunit
test for the function, as below.

    unsigned long foo(void)
    {
    	return 42;
    }

    static void foo_test(struct kunit *test)
    {
        KUNIT_EXPECT_EQ(test, 42, foo());
    }

However, this kunit gives me below warning for the above code:

    /.../linux/include/linux/kernel.h:842:29: warning: comparison of distinct pointer types lacks a cast
       (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
                                 ^
    /.../linux/include/kunit/test.h:493:9: note: in expansion of macro ‘__typecheck’
      ((void)__typecheck(__left, __right));           \
             ^~~~~~~~~~~
    /.../linux/include/kunit/test.h:517:2: note: in expansion of macro ‘KUNIT_BASE_BINARY_ASSERTION’
      KUNIT_BASE_BINARY_ASSERTION(test,           \
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~
    /.../linux/include/kunit/test.h:606:2: note: in expansion of macro ‘KUNIT_BASE_EQ_MSG_ASSERTION’
      KUNIT_BASE_EQ_MSG_ASSERTION(test,           \
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~
    /.../linux/include/kunit/test.h:616:2: note: in expansion of macro ‘KUNIT_BINARY_EQ_MSG_ASSERTION’
      KUNIT_BINARY_EQ_MSG_ASSERTION(test,           \
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    /.../linux/include/kunit/test.h:979:2: note: in expansion of macro ‘KUNIT_BINARY_EQ_ASSERTION’
      KUNIT_BINARY_EQ_ASSERTION(test, KUNIT_EXPECTATION, left, right)
      ^~~~~~~~~~~~~~~~~~~~~~~~~
    /.../linux/mm/foo-test.h:565:2: note: in expansion of macro ‘KUNIT_EXPECT_EQ’
      KUNIT_EXPECT_EQ(test, 42, foo());
      ^~~~~~~~~~~~~~~

I could remove the warning by explicitly type casting the constant as below:

        KUNIT_EXPECT_EQ(test, (unsigned long)42, foo());

However, now 'checkpatch.pl' complains about the type casting as below.

    WARNING: Unnecessary typecast of c90 int constant
    #565: FILE: mm/foo-test.h:565:
    +       KUNIT_EXPECT_EQ(test, (unsigned long)42, foo());

Of course, there could be several work-arounds for these warnings, such as
using 'EXPECT_TRUE(test, 42 == foo())' or casting the function's return value.
Nonetheless, I'm not sure what is the right way.  Could you please let me know
what is the recommended way for this case?


Thanks,
SeongJae Park

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-27 12:39 What is the best way to compare an unsigned and a constant? SeongJae Park
2019-12-27 12:52 ` Bernd Petrovitsch
2020-01-07 11:52   ` SeongJae Park
2020-01-07 13:35     ` Brendan Higgins
2020-01-07 13:49       ` SeongJae Park
2020-01-08 14:12         ` Brendan Higgins
2020-01-08 14:26           ` SeongJae Park

Linux-kselftest Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-kselftest/0 linux-kselftest/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-kselftest linux-kselftest/ https://lore.kernel.org/linux-kselftest \
		linux-kselftest@vger.kernel.org
	public-inbox-index linux-kselftest

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kselftest


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git