Fix implicit cast warning in test_klp_state.c

  The function `klp_get_state` returns an `int` value, but the variable
    `loglevel_state` is of type `struct klp_state *` and thus does an
    implicit cast. Explicitly casting these values fixes:

            - warning: assignment to ‘struct klp_state *’ from ‘int’
	    makes pointer from integer without a cast [-Wint-conversion]

    on lines 38, 55, 68 and 80 of test_klp_state.c

Signed-off-by: Shresth Prasad <shresthprasad7@gmail.com>
---
 .../selftests/livepatch/test_modules/test_klp_state.c     | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
  

On Sat, 17 Feb 2024 04:21:26 +0530 Shresth Prasad <shresthprasad7@gmail.com> wrote:

>     The function `klp_get_state` returns an `int` value, but the variable
>     `loglevel_state` is of type `struct klp_state *` and thus does an
>     implicit cast. Explicitly casting these values fixes:
> 
>             - warning: assignment to \u2018struct klp_state *\u2019 from \u2018int\u2019
> 	    makes pointer from integer without a cast [-Wint-conversion]
> 
>     on lines 38, 55, 68 and 80 of test_klp_state.c

I was unable to find where you saw the klp_get_state returning int. I tried
searching at the current master of live-patching repo[1], on linux-next. Can
you point where do you saw it? For me, klp_get_state return a pointer to klp_state.

Thanks,
  Marcos

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/livepatching/livepatching.git/tree/kernel/livepatch/state.c

> 
> Signed-off-by: Shresth Prasad <shresthprasad7@gmail.com>
> ---
>  .../selftests/livepatch/test_modules/test_klp_state.c     | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/livepatch/test_modules/test_klp_state.c b/tools/testing/selftests/livepatch/test_modules/test_klp_state.c
> index 57a4253acb01..ae6b1ca15fc0 100644
> --- a/tools/testing/selftests/livepatch/test_modules/test_klp_state.c
> +++ b/tools/testing/selftests/livepatch/test_modules/test_klp_state.c
> @@ -35,7 +35,7 @@ static int allocate_loglevel_state(void)
>  {
>  	struct klp_state *loglevel_state;
>  
> -	loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
> +	loglevel_state = (struct klp_state *)klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
>  	if (!loglevel_state)
>  		return -EINVAL;
>  
> @@ -52,7 +52,7 @@ static void fix_console_loglevel(void)
>  {
>  	struct klp_state *loglevel_state;
>  
> -	loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
> +	loglevel_state = (struct klp_state *)klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
>  	if (!loglevel_state)
>  		return;
>  
> @@ -65,7 +65,7 @@ static void restore_console_loglevel(void)
>  {
>  	struct klp_state *loglevel_state;
>  
> -	loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
> +	loglevel_state = (struct klp_state *)klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
>  	if (!loglevel_state)
>  		return;
>  
> @@ -77,7 +77,7 @@ static void free_loglevel_state(void)
>  {
>  	struct klp_state *loglevel_state;
>  
> -	loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
> +	loglevel_state = (struct klp_state *)klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
>  	if (!loglevel_state)
>  		return;
>  
> -- 
> 2.43.1
  

Well, the repo location I use is git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git.
It seem klp_get_state return struct klp_state.
The definition of this function in my repo as follows:

struct klp_state *klp_get_state(struct klp_patch *patch, unsigned long id) 
{
    struct klp_state *state;

    klp_for_each_state(patch, state) {
        if (state->id == id) 
            return state;
    }   

    return NULL;
}
EXPORT_SYMBOL_GPL(klp_get_state);

Are you sure there is really a need for forced conversion?

> 2024年2月19日 22:16，Marcos Paulo de Souza <mpdesouza@suse.com> 写道：
> 
> On Sat, 17 Feb 2024 04:21:26 +0530 Shresth Prasad <shresthprasad7@gmail.com> wrote:
> 
>>    The function `klp_get_state` returns an `int` value, but the variable
>>    `loglevel_state` is of type `struct klp_state *` and thus does an
>>    implicit cast. Explicitly casting these values fixes:
>> 
>>            - warning: assignment to \u2018struct klp_state *\u2019 from \u2018int\u2019
>>    makes pointer from integer without a cast [-Wint-conversion]
>> 
>>    on lines 38, 55, 68 and 80 of test_klp_state.c
> 
> I was unable to find where you saw the klp_get_state returning int. I tried
> searching at the current master of live-patching repo[1], on linux-next. Can
> you point where do you saw it? For me, klp_get_state return a pointer to klp_state.
> 
> Thanks,
>  Marcos
> 
> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/livepatching/livepatching.git/tree/kernel/livepatch/state.c
> 
>> 
>> Signed-off-by: Shresth Prasad <shresthprasad7@gmail.com>
>> ---
>> .../selftests/livepatch/test_modules/test_klp_state.c     | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>> 
>> diff --git a/tools/testing/selftests/livepatch/test_modules/test_klp_state.c b/tools/testing/selftests/livepatch/test_modules/test_klp_state.c
>> index 57a4253acb01..ae6b1ca15fc0 100644
>> --- a/tools/testing/selftests/livepatch/test_modules/test_klp_state.c
>> +++ b/tools/testing/selftests/livepatch/test_modules/test_klp_state.c
>> @@ -35,7 +35,7 @@ static int allocate_loglevel_state(void)
>> {
>> struct klp_state *loglevel_state;
>> 
>> - loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
>> + loglevel_state = (struct klp_state *)klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
>> if (!loglevel_state)
>> return -EINVAL;
>> 
>> @@ -52,7 +52,7 @@ static void fix_console_loglevel(void)
>> {
>> struct klp_state *loglevel_state;
>> 
>> - loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
>> + loglevel_state = (struct klp_state *)klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
>> if (!loglevel_state)
>> return;
>> 
>> @@ -65,7 +65,7 @@ static void restore_console_loglevel(void)
>> {
>> struct klp_state *loglevel_state;
>> 
>> - loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
>> + loglevel_state = (struct klp_state *)klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
>> if (!loglevel_state)
>> return;
>> 
>> @@ -77,7 +77,7 @@ static void free_loglevel_state(void)
>> {
>> struct klp_state *loglevel_state;
>> 
>> - loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
>> + loglevel_state = (struct klp_state *)klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
>> if (!loglevel_state)
>> return;
>> 
>> -- 
>> 2.43.1
>
  

Looking at the function definition now, I do see that the function returns a struct pointer but for me the compiler still complains about an implicit conversion from int to struct pointer.

Is there any particular reason why this might be happening? I couldn't quite figure it out myself as I am very new to working with the kernel.

Regards,
Shresth
  

On Tue, 20 Feb 2024 17:23:49 +0530 (GMT+05:30) Shresth Prasad <shresthprasad7@gmail.com> wrote:

> Looking at the function definition now, I do see that the function returns a struct pointer but for me the compiler still complains about an implicit conversion from int to struct pointer.
> 
> Is there any particular reason why this might be happening? I couldn't quite figure it out myself as I am very new to working with the kernel.

What compiler version and architecture? Are you compiling using flags like W=1?
I would advise you to always add more information about how the problem
manifests, and what you are executing. This can help to nail down the issue
quicker.

Thanks,
  Marcos

> 
> Regards,
> Shresth
  

>What compiler version and architecture? Are you >compiling using flags like W=1?
>I would advise you to always add more information >about how the problem
>manifests, and what you are executing. This can >help to nail down the issue quicker.

I'll keep that in mind. I'm on an x86_64 system with gcc version 13.2.1 20230801.

I'm using the command `make -j15 -C tools/testing/selftests` with no additional flags.

Regards,
Shresth
  

Would you please pasting the original warning of your complier? 
And did you check your source code if your source code is the the latest version?

Regards,
Warden

> On Feb 20, 2024, at 21:20, Shresth Prasad <shresthprasad7@gmail.com> wrote:
> 
>> What compiler version and architecture? Are you >compiling using flags like W=1?
>> I would advise you to always add more information >about how the problem
>> manifests, and what you are executing. This can >help to nail down the issue quicker.
> 
> I'll keep that in mind. I'm on an x86_64 system with gcc version 13.2.1 20230801.
> 
> I'm using the command `make -j15 -C tools/testing/selftests` with no additional flags.
> 
> Regards,
> Shresth
  

I checked the source code and yes I am on the latest Linux next repo.

Here's the warning:
/home/shresthp/dev/linux_work/linux_next/tools/testing/selftests/livepatch/test_modules/test_klp_state.c:38:24: warning: assignment to ‘struct klp_state *’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion]
   38 |         loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
      |                        ^
/home/shresthp/dev/linux_work/linux_next/tools/testing/selftests/livepatch/test_modules/test_klp_state.c: In function ‘fix_console_loglevel’:
/home/shresthp/dev/linux_work/linux_next/tools/testing/selftests/livepatch/test_modules/test_klp_state.c:55:24: warning: assignment to ‘struct klp_state *’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion]
   55 |         loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
      |                        ^
/home/shresthp/dev/linux_work/linux_next/tools/testing/selftests/livepatch/test_modules/test_klp_state.c: In function ‘restore_console_loglevel’:
/home/shresthp/dev/linux_work/linux_next/tools/testing/selftests/livepatch/test_modules/test_klp_state.c:68:24: warning: assignment to ‘struct klp_state *’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion]
   68 |         loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
      |                        ^
/home/shresthp/dev/linux_work/linux_next/tools/testing/selftests/livepatch/test_modules/test_klp_state.c: In function ‘free_loglevel_state’:
/home/shresthp/dev/linux_work/linux_next/tools/testing/selftests/livepatch/test_modules/test_klp_state.c:80:24: warning: assignment to ‘struct klp_state *’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion]
   80 |         loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
      |                        ^

Thank you for your help so far.

Regards,
Shresth
  

Shresth Prasad <shresthprasad7@gmail.com> writes:

> I checked the source code and yes I am on the latest Linux next repo.
>
> Here's the warning:
> /home/shresthp/dev/linux_work/linux_next/tools/testing/selftests/livepatch/test_modules/test_klp_state.c:38:24: warning: assignment to ‘struct klp_state *’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion]
>    38 |         loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
>       |                        ^


Is the declaration of klp_get_state() visible at that point, i.e. is
there perhaps any warning about missing declarations above that?

Otherwise C rules would default to assume an 'int' return type.

Thanks,

Nicolai
  

On 2/21/24 07:44, Nicolai Stange wrote:
> Shresth Prasad <shresthprasad7@gmail.com> writes:
> 
>> I checked the source code and yes I am on the latest Linux next repo.
>>
>> Here's the warning:
>> /home/shresthp/dev/linux_work/linux_next/tools/testing/selftests/livepatch/test_modules/test_klp_state.c:38:24: warning: assignment to ‘struct klp_state *’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion]
>>    38 |         loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
>>       |                        ^
> 
> 
> Is the declaration of klp_get_state() visible at that point, i.e. is
> there perhaps any warning about missing declarations above that?
> 
> Otherwise C rules would default to assume an 'int' return type.
> 

This is an interesting clue.  I thought I might be able to reproduce the
build error by modifying include/livepatch.h and running `make -j15 -C
tools/testing/selftests/livepatch` ... but that seemed to work fine on
my system.  I even removed the entire include/ subdir from my tree and
it still built the test module.  Huh?

Then I moved /lib/modules/$(uname -r)/build out of the way and saw that
the compilation failed.  Ah hah -- that's right, it's using the system
build tree.  That version of livepatch.h may have a missing or
completely different definition of klp_get_state().

How does this sequence work for you, Shresth:

  # Verify that kernel livepatching is turned on
  $ grep LIVEPATCH .config
  CONFIG_HAVE_LIVEPATCH=y
  CONFIG_LIVEPATCH=y

  # Build linux-next kernel tree and then the livepatch selftests,
  # pointing KDIR to this tree
  $ make -j$(nproc) vmlinux && \
    make -j$(nproc) KDIR=$(pwd) -C tools/testing/selftests/livepatch