| Message ID | 20240216225126.454999-1-shresthprasad7@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Fix implicit cast warning in test_klp_state.c | expand |
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
>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. I wasn't aware it works like that. You're right I do see some warnings about implicit function declarations of klp_get_state. Regards, Shresth
That sequence of steps fixed the warnings. Thank you so much! Regards, Shresth
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;
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(-)