With W=1 we see the following warning:
| drivers/char/ipmi/ipmi_si_platform.c:272:15: error: \
| cast to smaller integer type 'enum si_type' from \
| 'const void *' [-Werror,-Wvoid-pointer-to-enum-cast]
| 272 | io.si_type = (enum si_type) match->data;
| | ^~~~~~~~~~~~~~~~~~~~~~~~~~
This is due to the fact that the `si_type` enum members are int-width
and a cast from pointer-width down to int will cause truncation and
possible data loss. Although in this case `si_type` has only a few
enumerated fields and thus there is likely no data loss occurring.
Nonetheless, this patch is necessary to the goal of promoting this
warning out of W=1.
Link: https://github.com/ClangBuiltLinux/linux/issues/1902
Link: https://lore.kernel.org/llvm/[email protected]/
Reported-by: kernel test robot <[email protected]>
Signed-off-by: Justin Stitt <[email protected]>
---
Note:
Arnd had mentioned that there perhaps may be some semantic differences
between GCC and Clang regarding this warning or family of warnings. For
now, this patch (and others following) will yield less noisy W=1 builds
and hopefully materialize into this warning getting promoted out of W=1
to an always-on warning.
---
drivers/char/ipmi/ipmi_si_platform.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/char/ipmi/ipmi_si_platform.c b/drivers/char/ipmi/ipmi_si_platform.c
index 505cc978c97a..0d509d683c0f 100644
--- a/drivers/char/ipmi/ipmi_si_platform.c
+++ b/drivers/char/ipmi/ipmi_si_platform.c
@@ -269,7 +269,7 @@ static int of_ipmi_probe(struct platform_device *pdev)
}
memset(&io, 0, sizeof(io));
- io.si_type = (enum si_type) match->data;
+ io.si_type = (unsigned long) match->data;
io.addr_source = SI_DEVICETREE;
io.irq_setup = ipmi_std_irq_setup;
---
base-commit: c1a515d3c0270628df8ae5f5118ba859b85464a2
change-id: 20230809-cbl-1902-7532a747b731
Best regards,
--
Justin Stitt <[email protected]>
On Wed, Aug 09, 2023 at 09:05:17PM +0000, Justin Stitt wrote:
> With W=1 we see the following warning:
>
> | drivers/char/ipmi/ipmi_si_platform.c:272:15: error: \
> | cast to smaller integer type 'enum si_type' from \
> | 'const void *' [-Werror,-Wvoid-pointer-to-enum-cast]
> | 272 | io.si_type = (enum si_type) match->data;
> | | ^~~~~~~~~~~~~~~~~~~~~~~~~~
>
> This is due to the fact that the `si_type` enum members are int-width
> and a cast from pointer-width down to int will cause truncation and
> possible data loss. Although in this case `si_type` has only a few
> enumerated fields and thus there is likely no data loss occurring.
> Nonetheless, this patch is necessary to the goal of promoting this
> warning out of W=1.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1902
> Link: https://lore.kernel.org/llvm/[email protected]/
> Reported-by: kernel test robot <[email protected]>
> Signed-off-by: Justin Stitt <[email protected]>
> ---
> Note:
> Arnd had mentioned that there perhaps may be some semantic differences
> between GCC and Clang regarding this warning or family of warnings. For
> now, this patch (and others following) will yield less noisy W=1 builds
> and hopefully materialize into this warning getting promoted out of W=1
> to an always-on warning.
> ---
> drivers/char/ipmi/ipmi_si_platform.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/char/ipmi/ipmi_si_platform.c b/drivers/char/ipmi/ipmi_si_platform.c
> index 505cc978c97a..0d509d683c0f 100644
> --- a/drivers/char/ipmi/ipmi_si_platform.c
> +++ b/drivers/char/ipmi/ipmi_si_platform.c
> @@ -269,7 +269,7 @@ static int of_ipmi_probe(struct platform_device *pdev)
> }
>
> memset(&io, 0, sizeof(io));
> - io.si_type = (enum si_type) match->data;
> + io.si_type = (unsigned long) match->data;
Wouldn't you want to use intptr_t or uintptr_t?
-corey
> io.addr_source = SI_DEVICETREE;
> io.irq_setup = ipmi_std_irq_setup;
>
>
> ---
> base-commit: c1a515d3c0270628df8ae5f5118ba859b85464a2
> change-id: 20230809-cbl-1902-7532a747b731
>
> Best regards,
> --
> Justin Stitt <[email protected]>
>
From: Corey Minyard
> Sent: 11 August 2023 21:27
...
> > memset(&io, 0, sizeof(io));
> > - io.si_type = (enum si_type) match->data;
> > + io.si_type = (unsigned long) match->data;
>
> Wouldn't you want to use intptr_t or uintptr_t?
The kernel tends to use 'long' for the same reason
it doesn't use uint8_t.
Although I'm sure the correct/better fix is to either add variants
of the match/lookup functions that return (say) unsigned long
or change all the drivers to allocate a structure that contains
the required value.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)