On 3/30/22 07:50, David Laight wrote:
From: Guenter Roeck
Sent: 30 March 2022 15:23
On 3/29/22 09:07, 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
+underscore.
That needs more details and deserves its own paragraph. Calling one of
the functions is only necessary if the original name does or can include
unsupported characters; an unconditional "should" is therefore a bit
strong. Also, it is important to mention that the function duplicates
the name, and that it is the responsibility of the caller to release
the name if hwmon_sanitize_name() was called and the device is removed.
More worrying, and not documented, is that the buffer 'name' points
to must persist.
You mean the name argument passed to the hwmon registration functions ?
That has been the case forever, and I don't recall a single problem
with it. If it disturbs you, please feel free to submit a patch adding
more details to the documentation.
I would not want to change the code and always copy the name because in
almost all cases it _is_ a fixed string, and duplicating it would have
no value.
ISTM that the kmalloc() in __hwmon_device_register() should include
space for a copy of the name.
It can then do what it will with whatever is passed in.
Whatever is passed in is what the user wants. Registration functions
don't change the name. Providing a valid name is the responsibility
of the caller.
Oh yes, it has my 'favourite construct': if (!strlen(name)) ...
(well str[strlen(str)] = 0 also happens!)
Sorry, I don't understand what the problem is here.
Thanks,
Guenter
