Re: [PATCH v3] usb: core: sysfs: convert sysfs snprintf to sysfs_emit

From: Joe Perches
Date: Fri Jun 24 2022 - 08:37:18 EST


On Fri, 2022-06-24 at 20:12 +0800, Xuezhi Zhang wrote:
> Fix up all sysfs show entries to use sysfs_emit

Thanks.

Some trivia: (perhaps for a separate patch)

> diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c
[]
> @@ -35,7 +35,7 @@ static ssize_t field##_show(struct device *dev, \
> return -EINTR; \
> actconfig = udev->actconfig; \
> if (actconfig) \
> - rc = sprintf(buf, format_string, \
> + rc = sysfs_emit(buf, format_string, \
> actconfig->desc.field); \

It's much more common to use fmt and not format_string

Using fmt would make several multi-line statements fit on a single line

e.g.:

rc = sysfs_emit(buf, fmt, actconfig->desc.field); \

> @@ -305,8 +305,8 @@ static ssize_t ltm_capable_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> if (usb_device_supports_ltm(to_usb_device(dev)))
> - return sprintf(buf, "%s\n", "yes");
> - return sprintf(buf, "%s\n", "no");
> + return sysfs_emit(buf, "%s\n", "yes");
> + return sysfs_emit(buf, "%s\n", "no");

Using a ?: might be nicer

return sysfs_emit(buf, "%s\n",
usb_device_supports_ltm(to_usb_device(dev)) ? "yes" : "no");

> static ssize_t persist_store(struct device *dev, struct device_attribute *attr,
> @@ -372,7 +372,7 @@ static ssize_t connected_duration_show(struct device *dev,
> {
> struct usb_device *udev = to_usb_device(dev);
>
> - return sprintf(buf, "%u\n",
> + return sysfs_emit(buf, "%u\n",
> jiffies_to_msecs(jiffies - udev->connect_time));

Might be nicer to rewrap multi-line statements to the open parenthesis

return sysfs_emit(buf, "%u\n",
jiffies_to_msecs(jiffies - udev->connect_time));