| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <Y4jZ+RPZqvo1gyW/@nixie71>
Date: Thu, 1 Dec 2022 10:44:41 -0600
From: Jeff LaBundy <jeff@...undy.com>
To: ye.xingchen@....com.cn
Cc: dmitry.torokhov@...il.com, nick@...anahar.org,
giulio.benetti@...ettiengineering.com,
dario.binacchi@...rulasolutions.com, michael@...rulasolutions.com,
oliver.graute@...oconnector.com, wsa+renesas@...g-engineering.com,
u.kleine-koenig@...gutronix.de, johan@...nel.org,
linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] Input: touchscreen: use sysfs_emit() to instead of scnprintf()
Hi Ye,
On Thu, Dec 01, 2022 at 07:04:19PM +0800, ye.xingchen@....com.cn wrote:
> From: ye xingchen <ye.xingchen@....com.cn>
>
> Replace the open-code with sysfs_emit() to simplify the code.
>
> Signed-off-by: ye xingchen <ye.xingchen@....com.cn>
My comment is arguably cosmetic, but this patch distorts all of the
vertical alignment in these drivers. Example below.
> ---
> v1 -> v2
> fix the Subject.
> drivers/input/touchscreen/atmel_mxt_ts.c | 4 ++--
> drivers/input/touchscreen/edt-ft5x06.c | 2 +-
> drivers/input/touchscreen/hideep.c | 4 ++--
> drivers/input/touchscreen/hycon-hy46xx.c | 2 +-
> drivers/input/touchscreen/ilitek_ts_i2c.c | 4 ++--
> drivers/input/touchscreen/iqs5xx.c | 2 +-
> drivers/input/touchscreen/usbtouchscreen.c | 2 +-
> drivers/input/touchscreen/wdt87xx_i2c.c | 6 +++---
> 8 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index ccecd1441f0b..daad47c7823f 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -2761,7 +2761,7 @@ static ssize_t mxt_fw_version_show(struct device *dev,
> {
> struct mxt_data *data = dev_get_drvdata(dev);
> struct mxt_info *info = data->info;
> - return scnprintf(buf, PAGE_SIZE, "%u.%u.%02X\n",
> + return sysfs_emit(buf, "%u.%u.%02X\n",
> info->version >> 4, info->version & 0xf, info->build);
When applied locally, this apperas as:
return sysfs_emit(buf, ...
info->family_id,...
The original authors, myself included, spaced the second line over
just enough to be aligned and hence more readable. Please maintain
the same here.
In some cases, you may also be able to avoid a line break now that
PAGE_SIZE is no longer taking up space.
> }
>
> @@ -2771,7 +2771,7 @@ static ssize_t mxt_hw_version_show(struct device *dev,
> {
> struct mxt_data *data = dev_get_drvdata(dev);
> struct mxt_info *info = data->info;
> - return scnprintf(buf, PAGE_SIZE, "%u.%u\n",
> + return sysfs_emit(buf, "%u.%u\n",
> info->family_id, info->variant_id);
> }
>
> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> index 9ac1378610bc..b2ec2e04f943 100644
> --- a/drivers/input/touchscreen/edt-ft5x06.c
> +++ b/drivers/input/touchscreen/edt-ft5x06.c
> @@ -445,7 +445,7 @@ static ssize_t edt_ft5x06_setting_show(struct device *dev,
> *field = val;
> }
>
> - count = scnprintf(buf, PAGE_SIZE, "%d\n", val);
> + count = sysfs_emit(buf, "%d\n", val);
> out:
> mutex_unlock(&tsdata->mutex);
> return error ?: count;
> diff --git a/drivers/input/touchscreen/hideep.c b/drivers/input/touchscreen/hideep.c
> index e9547ee29756..c0d72ba3c44a 100644
> --- a/drivers/input/touchscreen/hideep.c
> +++ b/drivers/input/touchscreen/hideep.c
> @@ -922,7 +922,7 @@ static ssize_t hideep_fw_version_show(struct device *dev,
> ssize_t len;
>
> mutex_lock(&ts->dev_mutex);
> - len = scnprintf(buf, PAGE_SIZE, "%04x\n",
> + len = sysfs_emit(buf, "%04x\n",
> be16_to_cpu(ts->dwz_info.release_ver));
> mutex_unlock(&ts->dev_mutex);
>
> @@ -937,7 +937,7 @@ static ssize_t hideep_product_id_show(struct device *dev,
> ssize_t len;
>
> mutex_lock(&ts->dev_mutex);
> - len = scnprintf(buf, PAGE_SIZE, "%04x\n",
> + len = sysfs_emit(buf, "%04x\n",
> be16_to_cpu(ts->dwz_info.product_id));
> mutex_unlock(&ts->dev_mutex);
>
> diff --git a/drivers/input/touchscreen/hycon-hy46xx.c b/drivers/input/touchscreen/hycon-hy46xx.c
> index 891d0430083e..2d34959cb510 100644
> --- a/drivers/input/touchscreen/hycon-hy46xx.c
> +++ b/drivers/input/touchscreen/hycon-hy46xx.c
> @@ -202,7 +202,7 @@ static ssize_t hycon_hy46xx_setting_show(struct device *dev,
> *field = val;
> }
>
> - count = scnprintf(buf, PAGE_SIZE, "%d\n", val);
> + count = sysfs_emit(buf, "%d\n", val);
>
> out:
> mutex_unlock(&tsdata->mutex);
> diff --git a/drivers/input/touchscreen/ilitek_ts_i2c.c b/drivers/input/touchscreen/ilitek_ts_i2c.c
> index c5d259c76adc..114dceb13b77 100644
> --- a/drivers/input/touchscreen/ilitek_ts_i2c.c
> +++ b/drivers/input/touchscreen/ilitek_ts_i2c.c
> @@ -512,7 +512,7 @@ static ssize_t firmware_version_show(struct device *dev,
> struct i2c_client *client = to_i2c_client(dev);
> struct ilitek_ts_data *ts = i2c_get_clientdata(client);
>
> - return scnprintf(buf, PAGE_SIZE,
> + return sysfs_emit(buf,
> "fw version: [%02X%02X.%02X%02X.%02X%02X.%02X%02X]\n",
> ts->firmware_ver[0], ts->firmware_ver[1],
> ts->firmware_ver[2], ts->firmware_ver[3],
> @@ -527,7 +527,7 @@ static ssize_t product_id_show(struct device *dev,
> struct i2c_client *client = to_i2c_client(dev);
> struct ilitek_ts_data *ts = i2c_get_clientdata(client);
>
> - return scnprintf(buf, PAGE_SIZE, "product id: [%04X], module: [%s]\n",
> + return sysfs_emit(buf, "product id: [%04X], module: [%s]\n",
> ts->mcu_ver, ts->product_id);
> }
> static DEVICE_ATTR_RO(product_id);
> diff --git a/drivers/input/touchscreen/iqs5xx.c b/drivers/input/touchscreen/iqs5xx.c
> index 34c4cca57d13..9119e6b5c44f 100644
> --- a/drivers/input/touchscreen/iqs5xx.c
> +++ b/drivers/input/touchscreen/iqs5xx.c
> @@ -943,7 +943,7 @@ static ssize_t fw_info_show(struct device *dev,
> if (!iqs5xx->dev_id_info.bl_status)
> return -ENODATA;
>
> - return scnprintf(buf, PAGE_SIZE, "%u.%u.%u.%u:%u.%u\n",
> + return sysfs_emit(buf, "%u.%u.%u.%u:%u.%u\n",
> be16_to_cpu(iqs5xx->dev_id_info.prod_num),
> be16_to_cpu(iqs5xx->dev_id_info.proj_num),
> iqs5xx->dev_id_info.major_ver,
> diff --git a/drivers/input/touchscreen/usbtouchscreen.c b/drivers/input/touchscreen/usbtouchscreen.c
> index d6d04b9f04fc..4b2606805240 100644
> --- a/drivers/input/touchscreen/usbtouchscreen.c
> +++ b/drivers/input/touchscreen/usbtouchscreen.c
> @@ -456,7 +456,7 @@ static ssize_t mtouch_firmware_rev_show(struct device *dev,
> struct usbtouch_usb *usbtouch = usb_get_intfdata(intf);
> struct mtouch_priv *priv = usbtouch->priv;
>
> - return scnprintf(output, PAGE_SIZE, "%1x.%1x\n",
> + return sysfs_emit(output, "%1x.%1x\n",
> priv->fw_rev_major, priv->fw_rev_minor);
> }
> static DEVICE_ATTR(firmware_rev, 0444, mtouch_firmware_rev_show, NULL);
> diff --git a/drivers/input/touchscreen/wdt87xx_i2c.c b/drivers/input/touchscreen/wdt87xx_i2c.c
> index 166edeb77776..8f1b45ec2618 100644
> --- a/drivers/input/touchscreen/wdt87xx_i2c.c
> +++ b/drivers/input/touchscreen/wdt87xx_i2c.c
> @@ -887,7 +887,7 @@ static ssize_t config_csum_show(struct device *dev,
> cfg_csum = wdt->param.xmls_id1;
> cfg_csum = (cfg_csum << 16) | wdt->param.xmls_id2;
>
> - return scnprintf(buf, PAGE_SIZE, "%x\n", cfg_csum);
> + return sysfs_emit(buf, "%x\n", cfg_csum);
> }
>
> static ssize_t fw_version_show(struct device *dev,
> @@ -896,7 +896,7 @@ static ssize_t fw_version_show(struct device *dev,
> struct i2c_client *client = to_i2c_client(dev);
> struct wdt87xx_data *wdt = i2c_get_clientdata(client);
>
> - return scnprintf(buf, PAGE_SIZE, "%x\n", wdt->param.fw_id);
> + return sysfs_emit(buf, "%x\n", wdt->param.fw_id);
> }
>
> static ssize_t plat_id_show(struct device *dev,
> @@ -905,7 +905,7 @@ static ssize_t plat_id_show(struct device *dev,
> struct i2c_client *client = to_i2c_client(dev);
> struct wdt87xx_data *wdt = i2c_get_clientdata(client);
>
> - return scnprintf(buf, PAGE_SIZE, "%x\n", wdt->param.plat_id);
> + return sysfs_emit(buf, "%x\n", wdt->param.plat_id);
> }
>
> static ssize_t update_config_store(struct device *dev,
> --
> 2.25.1
Kind regards,
Jeff LaBundy
Powered by blists - more mailing lists