[PATCH] mtd: maps: fix -Wvoid-pointer-to-enum-cast warning

[PATCH] mtd: maps: fix -Wvoid-pointer-to-enum-cast warning

[Justin Stitt]

When building with clang 18 I see the following warning:
|       drivers/mtd/maps/physmap-versatile.c:209:25: warning: cast to smaller
|               integer type 'enum versatile_flashprot' from 'const void *' [-Wvoid-pointer-to-enum-cast]
|         209 |                 versatile_flashprot = (enum versatile_flashprot)devid->data;

This is due to the fact that `devid->data` is a void* while `enum
versatile_flashprot` has the size of an int. This leads to truncation
and possible data loss.

Link: https://github.com/ClangBuiltLinux/linux/issues/1910
Reported-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Justin Stitt <justinstitt@google.com>
---
Note: There is likely no data loss occurring here due to the fact that
`enum versatile_flashprot` has only a few enumerated fields, none of
which are large enough to cause data loss. Nonetheless, this patch helps
towards the goal of eventually enabling this warning for more builds.

Signed-off-by: Justin Stitt <justinstitt@google.com>
---
 drivers/mtd/maps/physmap-versatile.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/maps/physmap-versatile.c b/drivers/mtd/maps/physmap-versatile.c
index a1b8b7b25f88..d65cf8833771 100644
--- a/drivers/mtd/maps/physmap-versatile.c
+++ b/drivers/mtd/maps/physmap-versatile.c
@@ -206,7 +206,7 @@ int of_flash_probe_versatile(struct platform_device *pdev,
 		if (!sysnp)
 			return -ENODEV;
 
-		versatile_flashprot = (enum versatile_flashprot)devid->data;
+		versatile_flashprot = (uintptr_t)devid->data;
 		rmap = syscon_node_to_regmap(sysnp);
 		of_node_put(sysnp);
 		if (IS_ERR(rmap))

---
base-commit: 2ccdd1b13c591d306f0401d98dedc4bdcd02b421
change-id: 20230815-void-drivers-mtd-maps-physmap-versatile-2270fe7fdf16

Best regards,
--
Justin Stitt <justinstitt@google.com>


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: maps: fix -Wvoid-pointer-to-enum-cast warning

[Krzysztof Kozlowski]

On 15/08/2023 23:11, Justin Stitt wrote:
> When building with clang 18 I see the following warning:
> |       drivers/mtd/maps/physmap-versatile.c:209:25: warning: cast to smaller
> |               integer type 'enum versatile_flashprot' from 'const void *' [-Wvoid-pointer-to-enum-cast]
> |         209 |                 versatile_flashprot = (enum versatile_flashprot)devid->data;
> 
> This is due to the fact that `devid->data` is a void* while `enum
> versatile_flashprot` has the size of an int. This leads to truncation
> and possible data loss.

Cast does not solve truncation. This part of commit msg suggests that
you actually fix real issue... and that is an issue, because then
AUTOSEL will grab it. This is just compiler warning silencing and rather
coding standard correctness, no real fix, so please drop the sentence.

> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/1910
> Reported-by: Nathan Chancellor <nathan@kernel.org>
> Signed-off-by: Justin Stitt <justinstitt@google.com>
> ---
> Note: There is likely no data loss occurring here due to the fact that
> `enum versatile_flashprot` has only a few enumerated fields, none of
> which are large enough to cause data loss. 

If there is a data loss, cast does not solve it.

> Nonetheless, this patch helps
> towards the goal of eventually enabling this warning for more builds.
> 
> Signed-off-by: Justin Stitt <justinstitt@google.com>


Best regards,
Krzysztof


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: maps: fix -Wvoid-pointer-to-enum-cast warning

[Justin Stitt]

On Tue, Aug 15, 2023 at 2:15 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 15/08/2023 23:11, Justin Stitt wrote:
> > When building with clang 18 I see the following warning:
> > |       drivers/mtd/maps/physmap-versatile.c:209:25: warning: cast to smaller
> > |               integer type 'enum versatile_flashprot' from 'const void *' [-Wvoid-pointer-to-enum-cast]
> > |         209 |                 versatile_flashprot = (enum versatile_flashprot)devid->data;
> >
> > This is due to the fact that `devid->data` is a void* while `enum
> > versatile_flashprot` has the size of an int. This leads to truncation
> > and possible data loss.
>
> Cast does not solve truncation. This part of commit msg suggests that
> you actually fix real issue... and that is an issue, because then
> AUTOSEL will grab it. This is just compiler warning silencing and rather
> coding standard correctness, no real fix, so please drop the sentence.
OK, makes sense about this not technically solving an issue and thus
AUTOSEL may pick it up. Can you elaborate, though, on how the cast
doesn't solve truncation. Is the initial implementation not a
pointer-width down to int-width cast? Surely we're losing the top half
of bits. I'm still not saying there's data loss, to be clear. Just
that the compiler is warning because of the truncation.

>
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/1910
> > Reported-by: Nathan Chancellor <nathan@kernel.org>
> > Signed-off-by: Justin Stitt <justinstitt@google.com>
> > ---
> > Note: There is likely no data loss occurring here due to the fact that
> > `enum versatile_flashprot` has only a few enumerated fields, none of
> > which are large enough to cause data loss.
>
> If there is a data loss, cast does not solve it.
>
> > Nonetheless, this patch helps
> > towards the goal of eventually enabling this warning for more builds.
> >
> > Signed-off-by: Justin Stitt <justinstitt@google.com>
>
>
> Best regards,
> Krzysztof
>

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: maps: fix -Wvoid-pointer-to-enum-cast warning

[Krzysztof Kozlowski]

On 16/08/2023 01:06, Justin Stitt wrote:
> On Tue, Aug 15, 2023 at 2:15 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 15/08/2023 23:11, Justin Stitt wrote:
>>> When building with clang 18 I see the following warning:
>>> |       drivers/mtd/maps/physmap-versatile.c:209:25: warning: cast to smaller
>>> |               integer type 'enum versatile_flashprot' from 'const void *' [-Wvoid-pointer-to-enum-cast]
>>> |         209 |                 versatile_flashprot = (enum versatile_flashprot)devid->data;
>>>
>>> This is due to the fact that `devid->data` is a void* while `enum
>>> versatile_flashprot` has the size of an int. This leads to truncation
>>> and possible data loss.
>>
>> Cast does not solve truncation. This part of commit msg suggests that
>> you actually fix real issue... and that is an issue, because then
>> AUTOSEL will grab it. This is just compiler warning silencing and rather
>> coding standard correctness, no real fix, so please drop the sentence.
> OK, makes sense about this not technically solving an issue and thus
> AUTOSEL may pick it up. Can you elaborate, though, on how the cast
> doesn't solve truncation. 

Because that is no how the C language work?

Casting UINTMAX+1 to unsigned int, does not magically change the
unsigned int into something else...


> Is the initial implementation not a
> pointer-width down to int-width cast?

These are different widths, so cast cannot solve truncation.


> Surely we're losing the top half
> of bits. 

If we are losing top half then how is the truncation and data loss solved?

> I'm still not saying there's data loss, to be clear. Just
> that the compiler is warning because of the truncation.

Sorry, what truncation? The one which will happen always regardless of
the cast and warning?

Best regards,
Krzysztof


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/