Re: [PATCH 1/3] s390/vmlogrdr: Remove function pointer cast

[Heiko Carstens <hca@linux.ibm.com>]

On Thu, Apr 18, 2024 at 11:54:38AM +0200, Heiko Carstens wrote:
> On Wed, Apr 17, 2024 at 11:24:35AM -0700, Nathan Chancellor wrote:
> > Clang warns (or errors with CONFIG_WERROR) after enabling
> > -Wcast-function-type-strict by default:
> > 
> >   drivers/s390/char/vmlogrdr.c:746:18: error: cast from 'void (*)(const void *)' to 'void (*)(struct device *)' converts to incompatible function type [-Werror,-Wcast-function-type-strict]
> >     746 |                 dev->release = (void (*)(struct device *))kfree;
> >         |                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >   1 error generated.
> > 
> > Add a standalone function to fix the warning properly, which addresses
> > the root of the warning that these casts are not safe for kCFI. The
> > comment is not really relevant after this change, so remove it.
> > 
> > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> > ---
> >  drivers/s390/char/vmlogrdr.c | 13 +++++--------
> >  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> > @@ -736,14 +740,7 @@ static int vmlogrdr_register_device(struct vmlogrdr_priv_t *priv)
> >  		dev->driver = &vmlogrdr_driver;
> >  		dev->groups = vmlogrdr_attr_groups;
> >  		dev_set_drvdata(dev, priv);
> > -		/*
> > -		 * The release function could be called after the
> > -		 * module has been unloaded. It's _only_ task is to
> > -		 * free the struct. Therefore, we specify kfree()
> > -		 * directly here. (Probably a little bit obfuscating
> > -		 * but legitime ...).
> > -		 */
> 
> Why is the comment not relevant after this change? Or better: why is it not
> valid before this change, which is why the code was introduced a very long
> time ago? Any reference?
> 
> I've seen the warning since quite some time, but didn't change the code
> before sure that this doesn't introduce the bug described in the comment.

From only 20 years ago:

https://lore.kernel.org/all/20040316170812.GA14971@kroah.com/

The particular code (zfcp) was changed, so it doesn't have this code
(or never did?)  anymore, but for the rest this may or may not still
be valid.