Re: [PATCH] hid: replace snprintf in show functions with sysfs_emit

From: Bruno Prémont
Date: Fri Oct 15 2021 - 04:23:14 EST

Next message: Ming Qian: "[PATCH v11 00/13] amphion video decoder/encoder driver"
Previous message: Andy Shevchenko: "Re: [PATCH v7 1/2] hwmon: (asus_wmi_ec_sensors) Support B550 Asus WMI."
In reply to: Qing Wang: "[PATCH] hid: replace snprintf in show functions with sysfs_emit"
Next in thread: 王擎: "回复: [PATCH] hid: replace snprintf in show functions with sysfs_emit"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Thu, 14 Oct 2021 23:48:26 -0700 Qing Wang <wangqing@xxxxxxxx> wrote:
> show() must not use snprintf() when formatting the value to be
> returned to user space.
>
> Fix the coccicheck warnings:
> WARNING: use scnprintf or sprintf.
>
> Use sysfs_emit instead of scnprintf or sprintf makes more sense.

Looks good to me, but I have a few remarks:

- No need to talk about "must not use" in detailled commit message.
Mentioning introduction of the newish sysfs_emit() functions (in
commit 2efc459d06f1630001e3984854848a5647086232 a Year ago) and thus
switching over to those would be sufficient for code predating their
introduction.

- I'm wondering why picolcd_fb_update_rate_show() in
drivers/hid/hid-picolcd_fb.c:446
is not updated as well in this same patch.
There scnprintf() calls should be replaced with sysfs_emit_at() calls
according to the intent of this patch!

- Not sure if the patch should be split into a 5-patch series with one
patch per HID driver (each driver can be updated independently).

Acked-by: Bruno Prémont <bonbons@xxxxxxxxxxxxxxxxx>
(for picolcd code)

> Signed-off-by: Qing Wang <wangqing@xxxxxxxx>
> ---
> drivers/hid/hid-lenovo.c | 16 ++++++++--------
> drivers/hid/hid-picolcd_core.c | 6 +++---
> drivers/hid/hid-roccat-isku.c | 2 +-
> drivers/hid/hid-roccat-kone.c | 12 ++++++------
> drivers/hid/hid-roccat-koneplus.c | 4 ++--
> drivers/hid/hid-roccat-kovaplus.c | 10 +++++-----
> drivers/hid/hid-roccat-pyra.c | 6 +++---
> drivers/hid/hid-sensor-custom.c | 2 +-
> drivers/hid/hid-sony.c | 6 +++---
> 9 files changed, 32 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
> index 93b1f93..086a7ae 100644
> --- a/drivers/hid/hid-lenovo.c
> +++ b/drivers/hid/hid-lenovo.c
> @@ -400,7 +400,7 @@ static ssize_t attr_fn_lock_show(struct device *dev,
> struct hid_device *hdev = to_hid_device(dev);
> struct lenovo_drvdata *data = hid_get_drvdata(hdev);
>
> - return snprintf(buf, PAGE_SIZE, "%u\n", data->fn_lock);
> + return sysfs_emit(buf, "%u\n", data->fn_lock);
> }
>
> static ssize_t attr_fn_lock_store(struct device *dev,
> @@ -442,7 +442,7 @@ static ssize_t attr_sensitivity_show_cptkbd(struct device *dev,
> struct hid_device *hdev = to_hid_device(dev);
> struct lenovo_drvdata *cptkbd_data = hid_get_drvdata(hdev);
>
> - return snprintf(buf, PAGE_SIZE, "%u\n",
> + return sysfs_emit(buf, "%u\n",
> cptkbd_data->sensitivity);
> }
>
> @@ -603,7 +603,7 @@ static ssize_t attr_press_to_select_show_tpkbd(struct device *dev,
> struct hid_device *hdev = to_hid_device(dev);
> struct lenovo_drvdata *data_pointer = hid_get_drvdata(hdev);
>
> - return snprintf(buf, PAGE_SIZE, "%u\n", data_pointer->press_to_select);
> + return sysfs_emit(buf, "%u\n", data_pointer->press_to_select);
> }
>
> static ssize_t attr_press_to_select_store_tpkbd(struct device *dev,
> @@ -633,7 +633,7 @@ static ssize_t attr_dragging_show_tpkbd(struct device *dev,
> struct hid_device *hdev = to_hid_device(dev);
> struct lenovo_drvdata *data_pointer = hid_get_drvdata(hdev);
>
> - return snprintf(buf, PAGE_SIZE, "%u\n", data_pointer->dragging);
> + return sysfs_emit(buf, "%u\n", data_pointer->dragging);
> }
>
> static ssize_t attr_dragging_store_tpkbd(struct device *dev,
> @@ -663,7 +663,7 @@ static ssize_t attr_release_to_select_show_tpkbd(struct device *dev,
> struct hid_device *hdev = to_hid_device(dev);
> struct lenovo_drvdata *data_pointer = hid_get_drvdata(hdev);
>
> - return snprintf(buf, PAGE_SIZE, "%u\n", data_pointer->release_to_select);
> + return sysfs_emit(buf, "%u\n", data_pointer->release_to_select);
> }
>
> static ssize_t attr_release_to_select_store_tpkbd(struct device *dev,
> @@ -693,7 +693,7 @@ static ssize_t attr_select_right_show_tpkbd(struct device *dev,
> struct hid_device *hdev = to_hid_device(dev);
> struct lenovo_drvdata *data_pointer = hid_get_drvdata(hdev);
>
> - return snprintf(buf, PAGE_SIZE, "%u\n", data_pointer->select_right);
> + return sysfs_emit(buf, "%u\n", data_pointer->select_right);
> }
>
> static ssize_t attr_select_right_store_tpkbd(struct device *dev,
> @@ -723,7 +723,7 @@ static ssize_t attr_sensitivity_show_tpkbd(struct device *dev,
> struct hid_device *hdev = to_hid_device(dev);
> struct lenovo_drvdata *data_pointer = hid_get_drvdata(hdev);
>
> - return snprintf(buf, PAGE_SIZE, "%u\n",
> + return sysfs_emit(buf, "%u\n",
> data_pointer->sensitivity);
> }
>
> @@ -752,7 +752,7 @@ static ssize_t attr_press_speed_show_tpkbd(struct device *dev,
> struct hid_device *hdev = to_hid_device(dev);
> struct lenovo_drvdata *data_pointer = hid_get_drvdata(hdev);
>
> - return snprintf(buf, PAGE_SIZE, "%u\n",
> + return sysfs_emit(buf, "%u\n",
> data_pointer->press_speed);
> }
>
> diff --git a/drivers/hid/hid-picolcd_core.c b/drivers/hid/hid-picolcd_core.c
> index bbda231..fa46fb6 100644
> --- a/drivers/hid/hid-picolcd_core.c
> +++ b/drivers/hid/hid-picolcd_core.c
> @@ -256,9 +256,9 @@ static ssize_t picolcd_operation_mode_show(struct device *dev,
> struct picolcd_data *data = dev_get_drvdata(dev);
>
> if (data->status & PICOLCD_BOOTLOADER)
> - return snprintf(buf, PAGE_SIZE, "[bootloader] lcd\n");
> + return sysfs_emit(buf, "[bootloader] lcd\n");
> else
> - return snprintf(buf, PAGE_SIZE, "bootloader [lcd]\n");
> + return sysfs_emit(buf, "bootloader [lcd]\n");
> }
>
> static ssize_t picolcd_operation_mode_store(struct device *dev,
> @@ -301,7 +301,7 @@ static ssize_t picolcd_operation_mode_delay_show(struct device *dev,
> {
> struct picolcd_data *data = dev_get_drvdata(dev);
>
> - return snprintf(buf, PAGE_SIZE, "%hu\n", data->opmode_delay);
> + return sysfs_emit(buf, "%hu\n", data->opmode_delay);
> }
>
> static ssize_t picolcd_operation_mode_delay_store(struct device *dev,
> diff --git a/drivers/hid/hid-roccat-isku.c b/drivers/hid/hid-roccat-isku.c
> index ce5f225..58eb4b0 100644
> --- a/drivers/hid/hid-roccat-isku.c
> +++ b/drivers/hid/hid-roccat-isku.c
> @@ -63,7 +63,7 @@ static ssize_t isku_sysfs_show_actual_profile(struct device *dev,
> {
> struct isku_device *isku =
> hid_get_drvdata(dev_get_drvdata(dev->parent->parent));
> - return snprintf(buf, PAGE_SIZE, "%d\n", isku->actual_profile);
> + return sysfs_emit(buf, "%d\n", isku->actual_profile);
> }
>
> static ssize_t isku_sysfs_set_actual_profile(struct device *dev,
> diff --git a/drivers/hid/hid-roccat-kone.c b/drivers/hid/hid-roccat-kone.c
> index 1ca6448..66a7625 100644
> --- a/drivers/hid/hid-roccat-kone.c
> +++ b/drivers/hid/hid-roccat-kone.c
> @@ -403,7 +403,7 @@ static ssize_t kone_sysfs_show_actual_profile(struct device *dev,
> {
> struct kone_device *kone =
> hid_get_drvdata(dev_get_drvdata(dev->parent->parent));
> - return snprintf(buf, PAGE_SIZE, "%d\n", kone->actual_profile);
> + return sysfs_emit(buf, "%d\n", kone->actual_profile);
> }
> static DEVICE_ATTR(actual_profile, 0440, kone_sysfs_show_actual_profile, NULL);
>
> @@ -412,7 +412,7 @@ static ssize_t kone_sysfs_show_actual_dpi(struct device *dev,
> {
> struct kone_device *kone =
> hid_get_drvdata(dev_get_drvdata(dev->parent->parent));
> - return snprintf(buf, PAGE_SIZE, "%d\n", kone->actual_dpi);
> + return sysfs_emit(buf, "%d\n", kone->actual_dpi);
> }
> static DEVICE_ATTR(actual_dpi, 0440, kone_sysfs_show_actual_dpi, NULL);
>
> @@ -435,7 +435,7 @@ static ssize_t kone_sysfs_show_weight(struct device *dev,
>
> if (retval)
> return retval;
> - return snprintf(buf, PAGE_SIZE, "%d\n", weight);
> + return sysfs_emit(buf, "%d\n", weight);
> }
> static DEVICE_ATTR(weight, 0440, kone_sysfs_show_weight, NULL);
>
> @@ -444,7 +444,7 @@ static ssize_t kone_sysfs_show_firmware_version(struct device *dev,
> {
> struct kone_device *kone =
> hid_get_drvdata(dev_get_drvdata(dev->parent->parent));
> - return snprintf(buf, PAGE_SIZE, "%d\n", kone->firmware_version);
> + return sysfs_emit(buf, "%d\n", kone->firmware_version);
> }
> static DEVICE_ATTR(firmware_version, 0440, kone_sysfs_show_firmware_version,
> NULL);
> @@ -454,7 +454,7 @@ static ssize_t kone_sysfs_show_tcu(struct device *dev,
> {
> struct kone_device *kone =
> hid_get_drvdata(dev_get_drvdata(dev->parent->parent));
> - return snprintf(buf, PAGE_SIZE, "%d\n", kone->settings.tcu);
> + return sysfs_emit(buf, "%d\n", kone->settings.tcu);
> }
>
> static int kone_tcu_command(struct usb_device *usb_dev, int number)
> @@ -556,7 +556,7 @@ static ssize_t kone_sysfs_show_startup_profile(struct device *dev,
> {
> struct kone_device *kone =
> hid_get_drvdata(dev_get_drvdata(dev->parent->parent));
> - return snprintf(buf, PAGE_SIZE, "%d\n", kone->settings.startup_profile);
> + return sysfs_emit(buf, "%d\n", kone->settings.startup_profile);
> }
>
> static ssize_t kone_sysfs_set_startup_profile(struct device *dev,
> diff --git a/drivers/hid/hid-roccat-koneplus.c b/drivers/hid/hid-roccat-koneplus.c
> index 0316edf..9c39b17 100644
> --- a/drivers/hid/hid-roccat-koneplus.c
> +++ b/drivers/hid/hid-roccat-koneplus.c
> @@ -244,7 +244,7 @@ static ssize_t koneplus_sysfs_show_actual_profile(struct device *dev,
> {
> struct koneplus_device *koneplus =
> hid_get_drvdata(dev_get_drvdata(dev->parent->parent));
> - return snprintf(buf, PAGE_SIZE, "%d\n", koneplus->actual_profile);
> + return sysfs_emit(buf, "%d\n", koneplus->actual_profile);
> }
>
> static ssize_t koneplus_sysfs_set_actual_profile(struct device *dev,
> @@ -311,7 +311,7 @@ static ssize_t koneplus_sysfs_show_firmware_version(struct device *dev,
> &info, KONEPLUS_SIZE_INFO);
> mutex_unlock(&koneplus->koneplus_lock);
>
> - return snprintf(buf, PAGE_SIZE, "%d\n", info.firmware_version);
> + return sysfs_emit(buf, "%d\n", info.firmware_version);
> }
> static DEVICE_ATTR(firmware_version, 0440,
> koneplus_sysfs_show_firmware_version, NULL);
> diff --git a/drivers/hid/hid-roccat-kovaplus.c b/drivers/hid/hid-roccat-kovaplus.c
> index 9600128..17581c4 100644
> --- a/drivers/hid/hid-roccat-kovaplus.c
> +++ b/drivers/hid/hid-roccat-kovaplus.c
> @@ -274,7 +274,7 @@ static ssize_t kovaplus_sysfs_show_actual_profile(struct device *dev,
> {
> struct kovaplus_device *kovaplus =
> hid_get_drvdata(dev_get_drvdata(dev->parent->parent));
> - return snprintf(buf, PAGE_SIZE, "%d\n", kovaplus->actual_profile);
> + return sysfs_emit(buf, "%d\n", kovaplus->actual_profile);
> }
>
> static ssize_t kovaplus_sysfs_set_actual_profile(struct device *dev,
> @@ -327,7 +327,7 @@ static ssize_t kovaplus_sysfs_show_actual_cpi(struct device *dev,
> {
> struct kovaplus_device *kovaplus =
> hid_get_drvdata(dev_get_drvdata(dev->parent->parent));
> - return snprintf(buf, PAGE_SIZE, "%d\n", kovaplus->actual_cpi);
> + return sysfs_emit(buf, "%d\n", kovaplus->actual_cpi);
> }
> static DEVICE_ATTR(actual_cpi, 0440, kovaplus_sysfs_show_actual_cpi, NULL);
>
> @@ -336,7 +336,7 @@ static ssize_t kovaplus_sysfs_show_actual_sensitivity_x(struct device *dev,
> {
> struct kovaplus_device *kovaplus =
> hid_get_drvdata(dev_get_drvdata(dev->parent->parent));
> - return snprintf(buf, PAGE_SIZE, "%d\n", kovaplus->actual_x_sensitivity);
> + return sysfs_emit(buf, "%d\n", kovaplus->actual_x_sensitivity);
> }
> static DEVICE_ATTR(actual_sensitivity_x, 0440,
> kovaplus_sysfs_show_actual_sensitivity_x, NULL);
> @@ -346,7 +346,7 @@ static ssize_t kovaplus_sysfs_show_actual_sensitivity_y(struct device *dev,
> {
> struct kovaplus_device *kovaplus =
> hid_get_drvdata(dev_get_drvdata(dev->parent->parent));
> - return snprintf(buf, PAGE_SIZE, "%d\n", kovaplus->actual_y_sensitivity);
> + return sysfs_emit(buf, "%d\n", kovaplus->actual_y_sensitivity);
> }
> static DEVICE_ATTR(actual_sensitivity_y, 0440,
> kovaplus_sysfs_show_actual_sensitivity_y, NULL);
> @@ -367,7 +367,7 @@ static ssize_t kovaplus_sysfs_show_firmware_version(struct device *dev,
> &info, KOVAPLUS_SIZE_INFO);
> mutex_unlock(&kovaplus->kovaplus_lock);
>
> - return snprintf(buf, PAGE_SIZE, "%d\n", info.firmware_version);
> + return sysfs_emit(buf, "%d\n", info.firmware_version);
> }
> static DEVICE_ATTR(firmware_version, 0440,
> kovaplus_sysfs_show_firmware_version, NULL);
> diff --git a/drivers/hid/hid-roccat-pyra.c b/drivers/hid/hid-roccat-pyra.c
> index 989927d..9cfa003 100644
> --- a/drivers/hid/hid-roccat-pyra.c
> +++ b/drivers/hid/hid-roccat-pyra.c
> @@ -286,7 +286,7 @@ static ssize_t pyra_sysfs_show_actual_cpi(struct device *dev,
> {
> struct pyra_device *pyra =
> hid_get_drvdata(dev_get_drvdata(dev->parent->parent));
> - return snprintf(buf, PAGE_SIZE, "%d\n", pyra->actual_cpi);
> + return sysfs_emit(buf, "%d\n", pyra->actual_cpi);
> }
> static DEVICE_ATTR(actual_cpi, 0440, pyra_sysfs_show_actual_cpi, NULL);
>
> @@ -303,7 +303,7 @@ static ssize_t pyra_sysfs_show_actual_profile(struct device *dev,
> &settings, PYRA_SIZE_SETTINGS);
> mutex_unlock(&pyra->pyra_lock);
>
> - return snprintf(buf, PAGE_SIZE, "%d\n", settings.startup_profile);
> + return sysfs_emit(buf, "%d\n", settings.startup_profile);
> }
> static DEVICE_ATTR(actual_profile, 0440, pyra_sysfs_show_actual_profile, NULL);
> static DEVICE_ATTR(startup_profile, 0440, pyra_sysfs_show_actual_profile, NULL);
> @@ -324,7 +324,7 @@ static ssize_t pyra_sysfs_show_firmware_version(struct device *dev,
> &info, PYRA_SIZE_INFO);
> mutex_unlock(&pyra->pyra_lock);
>
> - return snprintf(buf, PAGE_SIZE, "%d\n", info.firmware_version);
> + return sysfs_emit(buf, "%d\n", info.firmware_version);
> }
> static DEVICE_ATTR(firmware_version, 0440, pyra_sysfs_show_firmware_version,
> NULL);
> diff --git a/drivers/hid/hid-sensor-custom.c b/drivers/hid/hid-sensor-custom.c
> index 32c2306..a46481d6 100644
> --- a/drivers/hid/hid-sensor-custom.c
> +++ b/drivers/hid/hid-sensor-custom.c
> @@ -371,7 +371,7 @@ static ssize_t show_value(struct device *dev, struct device_attribute *attr,
> sizeof(struct hid_custom_usage_desc),
> usage_id_cmp);
> if (usage_desc)
> - return snprintf(buf, PAGE_SIZE, "%s\n",
> + return sysfs_emit(buf, "%s\n",
> usage_desc->desc);
> else
> return sprintf(buf, "not-specified\n");
> diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
> index b3722c5..709595e 100644
> --- a/drivers/hid/hid-sony.c
> +++ b/drivers/hid/hid-sony.c
> @@ -696,7 +696,7 @@ static ssize_t ds4_show_poll_interval(struct device *dev,
> struct hid_device *hdev = to_hid_device(dev);
> struct sony_sc *sc = hid_get_drvdata(hdev);
>
> - return snprintf(buf, PAGE_SIZE, "%i\n", sc->ds4_bt_poll_interval);
> + return sysfs_emit(buf, "%i\n", sc->ds4_bt_poll_interval);
> }
>
> static ssize_t ds4_store_poll_interval(struct device *dev,
> @@ -733,7 +733,7 @@ static ssize_t sony_show_firmware_version(struct device *dev,
> struct hid_device *hdev = to_hid_device(dev);
> struct sony_sc *sc = hid_get_drvdata(hdev);
>
> - return snprintf(buf, PAGE_SIZE, "0x%04x\n", sc->fw_version);
> + return sysfs_emit(buf, "0x%04x\n", sc->fw_version);
> }
>
> static DEVICE_ATTR(firmware_version, 0444, sony_show_firmware_version, NULL);
> @@ -745,7 +745,7 @@ static ssize_t sony_show_hardware_version(struct device *dev,
> struct hid_device *hdev = to_hid_device(dev);
> struct sony_sc *sc = hid_get_drvdata(hdev);
>
> - return snprintf(buf, PAGE_SIZE, "0x%04x\n", sc->hw_version);
> + return sysfs_emit(buf, "0x%04x\n", sc->hw_version);
> }
>
> static DEVICE_ATTR(hardware_version, 0444, sony_show_hardware_version, NULL);

Next message: Ming Qian: "[PATCH v11 00/13] amphion video decoder/encoder driver"
Previous message: Andy Shevchenko: "Re: [PATCH v7 1/2] hwmon: (asus_wmi_ec_sensors) Support B550 Asus WMI."
In reply to: Qing Wang: "[PATCH] hid: replace snprintf in show functions with sysfs_emit"
Next in thread: 王擎: "回复: [PATCH] hid: replace snprintf in show functions with sysfs_emit"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]