On 03/07/2017 01:08 AM, Jean Delvare wrote:
Hi Guenter,
On Mon, 6 Mar 2017 15:47:55 -0800, Guenter Roeck wrote:
On Mon, Mar 06, 2017 at 09:48:35PM +0100, Peter Hüwe wrote:
Hi Guenter,
I was wondering whether there was a particular reason why
hwmon_attr_show_string passes only an "empty" pointer(pointer) to the ops-
read_string function rather than the buffer itself?
Wouldn't this mean that in ops->read_string I'd have to reserve some space for
the value on the heap (and taking care to free it somewhere, since returning
an address on the stack is bad idea), instead of calling sprintf(buf, "%s\n",
s) directly?
With the current implementation I have to sprintf it into my local buffer and
you sprintf it again into the final buffer.
The idea was that the called code would return a pointer to a constant string,
ie one that isn't changing from call to call.
In that case, what about the following change?
Subject: hwmon: Constify str parameter of hwmon_ops->read_string
The read_string callback is supposed to retrieve a pointer to a
constant string.
Signed-off-by: Jean Delvare <jdelvare@xxxxxxx>
Makes sense. I'll add it to -next.
Thanks,
Guenter
---
drivers/hwmon/hwmon.c | 2 +-
include/linux/hwmon.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
--- linux-4.10.orig/drivers/hwmon/hwmon.c 2017-02-19 23:34:00.000000000 +0100
+++ linux-4.10/drivers/hwmon/hwmon.c 2017-03-07 08:22:27.784527968 +0100
@@ -186,7 +186,7 @@ static ssize_t hwmon_attr_show_string(st
char *buf)
{
struct hwmon_device_attribute *hattr = to_hwmon_attr(devattr);
- char *s;
+ const char *s;
int ret;
ret = hattr->ops->read_string(dev, hattr->type, hattr->attr,
--- linux-4.10.orig/include/linux/hwmon.h 2017-02-19 23:34:00.000000000 +0100
+++ linux-4.10/include/linux/hwmon.h 2017-03-07 08:21:28.247998585 +0100
@@ -336,7 +336,7 @@ struct hwmon_ops {
int (*read)(struct device *dev, enum hwmon_sensor_types type,
u32 attr, int channel, long *val);
int (*read_string)(struct device *dev, enum hwmon_sensor_types type,
- u32 attr, int channel, char **str);
+ u32 attr, int channel, const char **str);
int (*write)(struct device *dev, enum hwmon_sensor_types type,
u32 attr, int channel, long val);
};
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
