Re: [PATCH] tty: hvc-iucv: fix function pointer casts
On Wed, Feb 14, 2024 at 11:37:21AM +0100, Greg Kroah-Hartman wrote: > On Wed, Feb 14, 2024 at 09:46:33AM +, David Laight wrote: > > From: Segher Boessenkool > > > Sent: 13 February 2024 19:13 > > > > > > On Tue, Feb 13, 2024 at 11:17:49AM +0100, Arnd Bergmann wrote: > > > > clang warns about explicitly casting between incompatible function > > > > pointers: > > > > > > > > drivers/tty/hvc/hvc_iucv.c:1100:23: error: cast from 'void (*)(const > > > > void *)' to 'void (*)(struct > > > device *)' converts to incompatible function type > > > [-Werror,-Wcast-function-type-strict] > > > > 1100 | priv->dev->release = (void (*)(struct device *)) kfree; > > > > | ^ > > > > > > Such a cast of course is explicitly allowed by 6.3.2.3/8, only calling a > > > function using a non-compatible type is UB. This warning message is > > > quite misleading. Doubly so because of the -Werror, as always. > > > > But it will get called using the wrong type. > > And (is it) fine-ibt will reject the incorrect call. > > And rightfully so, this type of casting abuse is just that, abuse. > > Almost no one should be just calling kfree() on a device pointer, I'll > look at the surrounding code as odds are something odd is going on. But > for now, this patch is correct. Yes, and I said so. My point is that the warning message pretends the cast is bad or dangerous. It is not, similar casts are the only way in C to do certain things (yes, you can always avoid it completely, by writing completely different code, like the patch does, and that sometimes is a better idea even). But the warning message is misleading and does more damage than it helps avoid. Segher
Re: [PATCH] tty: hvc-iucv: fix function pointer casts
On Wed, Feb 14, 2024 at 09:46:33AM +, David Laight wrote: > From: Segher Boessenkool > > Sent: 13 February 2024 19:13 > > > > On Tue, Feb 13, 2024 at 11:17:49AM +0100, Arnd Bergmann wrote: > > > clang warns about explicitly casting between incompatible function > > > pointers: > > > > > > drivers/tty/hvc/hvc_iucv.c:1100:23: error: cast from 'void (*)(const void > > > *)' to 'void (*)(struct > > device *)' converts to incompatible function type > > [-Werror,-Wcast-function-type-strict] > > > 1100 | priv->dev->release = (void (*)(struct device *)) kfree; > > > | ^ > > > > Such a cast of course is explicitly allowed by 6.3.2.3/8, only calling a > > function using a non-compatible type is UB. This warning message is > > quite misleading. Doubly so because of the -Werror, as always. > > But it will get called using the wrong type. > And (is it) fine-ibt will reject the incorrect call. And rightfully so, this type of casting abuse is just that, abuse. Almost no one should be just calling kfree() on a device pointer, I'll look at the surrounding code as odds are something odd is going on. But for now, this patch is correct. thanks, greg k-h
RE: [PATCH] tty: hvc-iucv: fix function pointer casts
From: Segher Boessenkool > Sent: 13 February 2024 19:13 > > On Tue, Feb 13, 2024 at 11:17:49AM +0100, Arnd Bergmann wrote: > > clang warns about explicitly casting between incompatible function > > pointers: > > > > drivers/tty/hvc/hvc_iucv.c:1100:23: error: cast from 'void (*)(const void > > *)' to 'void (*)(struct > device *)' converts to incompatible function type > [-Werror,-Wcast-function-type-strict] > > 1100 | priv->dev->release = (void (*)(struct device *)) kfree; > > | ^ > > Such a cast of course is explicitly allowed by 6.3.2.3/8, only calling a > function using a non-compatible type is UB. This warning message is > quite misleading. Doubly so because of the -Werror, as always. But it will get called using the wrong type. And (is it) fine-ibt will reject the incorrect call. Has clang/gcc added an attribute to 'seed' the ibt hash yet? So that functions that are void (*)(void) can be separated? David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Re: [PATCH] tty: hvc-iucv: fix function pointer casts
On 13. 02. 24, 11:17, Arnd Bergmann wrote:
From: Arnd Bergmann
clang warns about explicitly casting between incompatible function
pointers:
drivers/tty/hvc/hvc_iucv.c:1100:23: error: cast from 'void (*)(const void *)'
to 'void (*)(struct device *)' converts to incompatible function type
[-Werror,-Wcast-function-type-strict]
1100 | priv->dev->release = (void (*)(struct device *)) kfree;
| ^
Add a separate function to handle this correctly.
Signed-off-by: Arnd Bergmann
Reviewed-by: Jiri Slaby
diff --git a/drivers/tty/hvc/hvc_iucv.c b/drivers/tty/hvc/hvc_iucv.c
index fdecc0d63731..b1149bc62ca1 100644
--- a/drivers/tty/hvc/hvc_iucv.c
+++ b/drivers/tty/hvc/hvc_iucv.c
@@ -1035,6 +1035,10 @@ static const struct attribute_group
*hvc_iucv_dev_attr_groups[] = {
NULL,
};
+static void hvc_iucv_free(struct device *data)
+{
+ kfree(data);
+}
/**
* hvc_iucv_alloc() - Allocates a new struct hvc_iucv_private instance
@@ -1097,7 +1101,7 @@ static int __init hvc_iucv_alloc(int id, unsigned int
is_console)
priv->dev->bus = &iucv_bus;
priv->dev->parent = iucv_root;
priv->dev->groups = hvc_iucv_dev_attr_groups;
- priv->dev->release = (void (*)(struct device *)) kfree;
+ priv->dev->release = hvc_iucv_free;
rc = device_register(priv->dev);
if (rc) {
put_device(priv->dev);
--
js
suse labs
Re: [PATCH] tty: hvc-iucv: fix function pointer casts
On Tue, Feb 13, 2024 at 11:17:49AM +0100, Arnd Bergmann wrote: > clang warns about explicitly casting between incompatible function > pointers: > > drivers/tty/hvc/hvc_iucv.c:1100:23: error: cast from 'void (*)(const void *)' > to 'void (*)(struct device *)' converts to incompatible function type > [-Werror,-Wcast-function-type-strict] > 1100 | priv->dev->release = (void (*)(struct device *)) kfree; > | ^ Such a cast of course is explicitly allowed by 6.3.2.3/8, only calling a function using a non-compatible type is UB. This warning message is quite misleading. Doubly so because of the -Werror, as always. Your proposed new code of course is nice and simple (albeit a bit bigger than it was before, both source and binary). Such is life ;-) Segher
[PATCH] tty: hvc-iucv: fix function pointer casts
From: Arnd Bergmann
clang warns about explicitly casting between incompatible function
pointers:
drivers/tty/hvc/hvc_iucv.c:1100:23: error: cast from 'void (*)(const void *)'
to 'void (*)(struct device *)' converts to incompatible function type
[-Werror,-Wcast-function-type-strict]
1100 | priv->dev->release = (void (*)(struct device *)) kfree;
| ^
Add a separate function to handle this correctly.
Signed-off-by: Arnd Bergmann
---
drivers/tty/hvc/hvc_iucv.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/hvc/hvc_iucv.c b/drivers/tty/hvc/hvc_iucv.c
index fdecc0d63731..b1149bc62ca1 100644
--- a/drivers/tty/hvc/hvc_iucv.c
+++ b/drivers/tty/hvc/hvc_iucv.c
@@ -1035,6 +1035,10 @@ static const struct attribute_group
*hvc_iucv_dev_attr_groups[] = {
NULL,
};
+static void hvc_iucv_free(struct device *data)
+{
+ kfree(data);
+}
/**
* hvc_iucv_alloc() - Allocates a new struct hvc_iucv_private instance
@@ -1097,7 +1101,7 @@ static int __init hvc_iucv_alloc(int id, unsigned int
is_console)
priv->dev->bus = &iucv_bus;
priv->dev->parent = iucv_root;
priv->dev->groups = hvc_iucv_dev_attr_groups;
- priv->dev->release = (void (*)(struct device *)) kfree;
+ priv->dev->release = hvc_iucv_free;
rc = device_register(priv->dev);
if (rc) {
put_device(priv->dev);
--
2.39.2
