On 3/30/22 07:51, Xu Yilun wrote:
On Wed, Mar 30, 2022 at 10:11:39AM +0000, David Laight wrote:
From: Xu Yilun
Sent: 30 March 2022 07:51
On Tue, Mar 29, 2022 at 06:07:26PM +0200, Michael Walle wrote:
More and more drivers will check for bad characters in the hwmon name
and all are using the same code snippet. Consolidate that code by adding
a new hwmon_sanitize_name() function.
Signed-off-by: Michael Walle <michael@xxxxxxxx>
---
Documentation/hwmon/hwmon-kernel-api.rst | 9 ++++-
drivers/hwmon/hwmon.c | 49 ++++++++++++++++++++++++
include/linux/hwmon.h | 3 ++
3 files changed, 60 insertions(+), 1 deletion(-)
diff --git a/Documentation/hwmon/hwmon-kernel-api.rst b/Documentation/hwmon/hwmon-kernel-api.rst
index c41eb6108103..12f4a9bcef04 100644
--- a/Documentation/hwmon/hwmon-kernel-api.rst
+++ b/Documentation/hwmon/hwmon-kernel-api.rst
@@ -50,6 +50,10 @@ register/unregister functions::
void devm_hwmon_device_unregister(struct device *dev);
+ char *hwmon_sanitize_name(const char *name);
+
+ char *devm_hwmon_sanitize_name(struct device *dev, const char *name);
+
hwmon_device_register_with_groups registers a hardware monitoring device.
The first parameter of this function is a pointer to the parent device.
The name parameter is a pointer to the hwmon device name. The registration
@@ -93,7 +97,10 @@ removal would be too late.
All supported hwmon device registration functions only accept valid device
names. Device names including invalid characters (whitespace, '*', or '-')
-will be rejected. The 'name' parameter is mandatory.
+will be rejected. The 'name' parameter is mandatory. Before calling a
+register function you should either use hwmon_sanitize_name or
+devm_hwmon_sanitize_name to replace any invalid characters with an
I suggest to duplicate the name and replace ...
You are now going to get code that passed in NULL when the kmalloc() fails.
If 'sanitizing' the name is the correct thing to do then sanitize it
when the copy is made into the allocated structure.
Then the driver is unaware of the name change, which makes more
confusing.
(I'm assuming that the 'const char *name' parameter doesn't have to
be persistent - that would be another bug just waiting to happen.)
The hwmon core does require a persistent "name" parameter now. No name
copy is made when hwmon dev register.
Seems really pointless to be do a kmalloc() just to pass a string
into a function.
Maybe we should not force a kmalloc() when the sanitizing is needed, let
the driver decide whether to duplicate the string or not.
Drivers can do that today, and in all existing cases they do so
(which is why I had suggested to handle the duplication in the
convenience function in the first place). Drivers don't _have_
to use the provided convenience functions. At the same time,
convenience functions should cover the most common use cases.
Michael, let's just drop the changes outside drivers/hwmon from
the series, and let's keep hwmon_is_bad_char() in the include file.
Let's just document it, explaining its use case.
Code outside drivers/hwmon can then be independently modified or not
at a later time, based on driver author and/or maintainer preference.
Thanks,
Guenter
