Re: [PATCH] platform/chrome: use sysfs_emit_at() instead of scnprintf()

[Guenter Roeck <linux@roeck-us.net>]

On Mon, Dec 05, 2022 at 02:10:42PM +0800, Tzung-Bi Shih wrote:
> Follow the advice in Documentation/filesystems/sysfs.rst:
> show() should only use sysfs_emit() or sysfs_emit_at() when formatting
> the value to be returned to user space.
> 
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> Inspired by https://patchwork.kernel.org/project/chrome-platform/patch/202212021656040995199@zte.com.cn/
> 
>  drivers/platform/chrome/cros_ec_sysfs.c | 36 ++++++++++---------------
>  1 file changed, 14 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/platform/chrome/cros_ec_sysfs.c b/drivers/platform/chrome/cros_ec_sysfs.c
> index e45e57cee3a8..09e3bf5e8ec6 100644
> --- a/drivers/platform/chrome/cros_ec_sysfs.c
> +++ b/drivers/platform/chrome/cros_ec_sysfs.c
> @@ -27,10 +27,9 @@ static ssize_t reboot_show(struct device *dev,
>  {
>  	int count = 0;
>  
> -	count += scnprintf(buf + count, PAGE_SIZE - count,
> -			   "ro|rw|cancel|cold|disable-jump|hibernate|cold-ap-off");
> -	count += scnprintf(buf + count, PAGE_SIZE - count,
> -			   " [at-shutdown]\n");
> +	count += sysfs_emit_at(buf, count,
> +			       "ro|rw|cancel|cold|disable-jump|hibernate|cold-ap-off");
> +	count += sysfs_emit_at(buf, count, " [at-shutdown]\n");
>  	return count;
>  }
>  
> @@ -138,12 +137,9 @@ static ssize_t version_show(struct device *dev,
>  	/* Strings should be null-terminated, but let's be sure. */
>  	r_ver->version_string_ro[sizeof(r_ver->version_string_ro) - 1] = '\0';
>  	r_ver->version_string_rw[sizeof(r_ver->version_string_rw) - 1] = '\0';
> -	count += scnprintf(buf + count, PAGE_SIZE - count,
> -			   "RO version:    %s\n", r_ver->version_string_ro);
> -	count += scnprintf(buf + count, PAGE_SIZE - count,
> -			   "RW version:    %s\n", r_ver->version_string_rw);
> -	count += scnprintf(buf + count, PAGE_SIZE - count,
> -			   "Firmware copy: %s\n",
> +	count += sysfs_emit_at(buf, count, "RO version:    %s\n", r_ver->version_string_ro);
> +	count += sysfs_emit_at(buf, count, "RW version:    %s\n", r_ver->version_string_rw);
> +	count += sysfs_emit_at(buf, count, "Firmware copy: %s\n",
>  			   (r_ver->current_image < ARRAY_SIZE(image_names) ?
>  			    image_names[r_ver->current_image] : "?"));
>  
> @@ -152,13 +148,12 @@ static ssize_t version_show(struct device *dev,
>  	msg->insize = EC_HOST_PARAM_SIZE;
>  	ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
>  	if (ret < 0) {
> -		count += scnprintf(buf + count, PAGE_SIZE - count,
> +		count += sysfs_emit_at(buf, count,
>  				   "Build info:    XFER / EC ERROR %d / %d\n",
>  				   ret, msg->result);
>  	} else {
>  		msg->data[EC_HOST_PARAM_SIZE - 1] = '\0';
> -		count += scnprintf(buf + count, PAGE_SIZE - count,
> -				   "Build info:    %s\n", msg->data);
> +		count += sysfs_emit_at(buf, count, "Build info:    %s\n", msg->data);
>  	}
>  
>  	/* Get chip info. */
> @@ -166,7 +161,7 @@ static ssize_t version_show(struct device *dev,
>  	msg->insize = sizeof(*r_chip);
>  	ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
>  	if (ret < 0) {
> -		count += scnprintf(buf + count, PAGE_SIZE - count,
> +		count += sysfs_emit_at(buf, count,
>  				   "Chip info:     XFER / EC ERROR %d / %d\n",
>  				   ret, msg->result);
>  	} else {
> @@ -175,12 +170,9 @@ static ssize_t version_show(struct device *dev,
>  		r_chip->vendor[sizeof(r_chip->vendor) - 1] = '\0';
>  		r_chip->name[sizeof(r_chip->name) - 1] = '\0';
>  		r_chip->revision[sizeof(r_chip->revision) - 1] = '\0';
> -		count += scnprintf(buf + count, PAGE_SIZE - count,
> -				   "Chip vendor:   %s\n", r_chip->vendor);
> -		count += scnprintf(buf + count, PAGE_SIZE - count,
> -				   "Chip name:     %s\n", r_chip->name);
> -		count += scnprintf(buf + count, PAGE_SIZE - count,
> -				   "Chip revision: %s\n", r_chip->revision);
> +		count += sysfs_emit_at(buf, count, "Chip vendor:   %s\n", r_chip->vendor);
> +		count += sysfs_emit_at(buf, count, "Chip name:     %s\n", r_chip->name);
> +		count += sysfs_emit_at(buf, count, "Chip revision: %s\n", r_chip->revision);
>  	}
>  
>  	/* Get board version */
> @@ -188,13 +180,13 @@ static ssize_t version_show(struct device *dev,
>  	msg->insize = sizeof(*r_board);
>  	ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
>  	if (ret < 0) {
> -		count += scnprintf(buf + count, PAGE_SIZE - count,
> +		count += sysfs_emit_at(buf, count,
>  				   "Board version: XFER / EC ERROR %d / %d\n",
>  				   ret, msg->result);
>  	} else {
>  		r_board = (struct ec_response_board_version *)msg->data;
>  
> -		count += scnprintf(buf + count, PAGE_SIZE - count,
> +		count += sysfs_emit_at(buf, count,
>  				   "Board version: %d\n",
>  				   r_board->board_version);
>  	}