2021-10-15 13:05:38

by 王擎

Subject: [PATCH] s390: replace snprintf in show functions with sysfs_emit

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.

Signed-off-by: Qing Wang <[email protected]>
---
drivers/s390/block/dasd_devmap.c | 74 ++++++++++++++++++++--------------------
drivers/s390/char/raw3270.c | 6 ++--
drivers/s390/cio/chp.c | 2 +-
3 files changed, 41 insertions(+), 41 deletions(-)

diff --git a/drivers/s390/block/dasd_devmap.c b/drivers/s390/block/dasd_devmap.c
index 2c40fe1..78eb479 100644
--- a/drivers/s390/block/dasd_devmap.c
+++ b/drivers/s390/block/dasd_devmap.c
@@ -731,7 +731,7 @@ static ssize_t dasd_ff_show(struct device *dev, struct device_attribute *attr,
ff_flag = (devmap->features & DASD_FEATURE_FAILFAST) != 0;
else
ff_flag = (DASD_FEATURE_DEFAULT & DASD_FEATURE_FAILFAST) != 0;
- return snprintf(buf, PAGE_SIZE, ff_flag ? "1\n" : "0\n");
+ return sysfs_emit(buf, ff_flag ? "1\n" : "0\n");
}

static ssize_t dasd_ff_store(struct device *dev, struct device_attribute *attr,
@@ -773,7 +773,7 @@ dasd_ro_show(struct device *dev, struct device_attribute *attr, char *buf)
spin_unlock(&dasd_devmap_lock);

out:
- return snprintf(buf, PAGE_SIZE, ro_flag ? "1\n" : "0\n");
+ return sysfs_emit(buf, ro_flag ? "1\n" : "0\n");
}

static ssize_t
@@ -834,7 +834,7 @@ dasd_erplog_show(struct device *dev, struct device_attribute *attr, char *buf)
erplog = (devmap->features & DASD_FEATURE_ERPLOG) != 0;
else
erplog = (DASD_FEATURE_DEFAULT & DASD_FEATURE_ERPLOG) != 0;
- return snprintf(buf, PAGE_SIZE, erplog ? "1\n" : "0\n");
+ return sysfs_emit(buf, erplog ? "1\n" : "0\n");
}

static ssize_t
@@ -1033,13 +1033,13 @@ dasd_discipline_show(struct device *dev, struct device_attribute *attr,
dasd_put_device(device);
goto out;
} else {
- len = snprintf(buf, PAGE_SIZE, "%s\n",
+ len = sysfs_emit(buf, "%s\n",
device->discipline->name);
dasd_put_device(device);
return len;
}
out:
- len = snprintf(buf, PAGE_SIZE, "none\n");
+ len = sysfs_emit(buf, "none\n");
return len;
}

@@ -1056,30 +1056,30 @@ dasd_device_status_show(struct device *dev, struct device_attribute *attr,
if (!IS_ERR(device)) {
switch (device->state) {
case DASD_STATE_NEW:
- len = snprintf(buf, PAGE_SIZE, "new\n");
+ len = sysfs_emit(buf, "new\n");
break;
case DASD_STATE_KNOWN:
- len = snprintf(buf, PAGE_SIZE, "detected\n");
+ len = sysfs_emit(buf, "detected\n");
break;
case DASD_STATE_BASIC:
- len = snprintf(buf, PAGE_SIZE, "basic\n");
+ len = sysfs_emit(buf, "basic\n");
break;
case DASD_STATE_UNFMT:
- len = snprintf(buf, PAGE_SIZE, "unformatted\n");
+ len = sysfs_emit(buf, "unformatted\n");
break;
case DASD_STATE_READY:
- len = snprintf(buf, PAGE_SIZE, "ready\n");
+ len = sysfs_emit(buf, "ready\n");
break;
case DASD_STATE_ONLINE:
- len = snprintf(buf, PAGE_SIZE, "online\n");
+ len = sysfs_emit(buf, "online\n");
break;
default:
- len = snprintf(buf, PAGE_SIZE, "no stat\n");
+ len = sysfs_emit(buf, "no stat\n");
break;
}
dasd_put_device(device);
} else
- len = snprintf(buf, PAGE_SIZE, "unknown\n");
+ len = sysfs_emit(buf, "unknown\n");
return len;
}

@@ -1120,7 +1120,7 @@ static ssize_t dasd_vendor_show(struct device *dev,
device = dasd_device_from_cdev(to_ccwdev(dev));
vendor = "";
if (IS_ERR(device))
- return snprintf(buf, PAGE_SIZE, "%s\n", vendor);
+ return sysfs_emit(buf, "%s\n", vendor);

if (device->discipline && device->discipline->get_uid &&
!device->discipline->get_uid(device, &uid))
@@ -1128,7 +1128,7 @@ static ssize_t dasd_vendor_show(struct device *dev,

dasd_put_device(device);

- return snprintf(buf, PAGE_SIZE, "%s\n", vendor);
+ return sysfs_emit(buf, "%s\n", vendor);
}

static DEVICE_ATTR(vendor, 0444, dasd_vendor_show, NULL);
@@ -1148,7 +1148,7 @@ dasd_uid_show(struct device *dev, struct device_attribute *attr, char *buf)
device = dasd_device_from_cdev(to_ccwdev(dev));
uid_string[0] = 0;
if (IS_ERR(device))
- return snprintf(buf, PAGE_SIZE, "%s\n", uid_string);
+ return sysfs_emit(buf, "%s\n", uid_string);

if (device->discipline && device->discipline->get_uid &&
!device->discipline->get_uid(device, &uid)) {
@@ -1183,7 +1183,7 @@ dasd_uid_show(struct device *dev, struct device_attribute *attr, char *buf)
}
dasd_put_device(device);

- return snprintf(buf, PAGE_SIZE, "%s\n", uid_string);
+ return sysfs_emit(buf, "%s\n", uid_string);
}
static DEVICE_ATTR(uid, 0444, dasd_uid_show, NULL);

@@ -1201,7 +1201,7 @@ dasd_eer_show(struct device *dev, struct device_attribute *attr, char *buf)
eer_flag = dasd_eer_enabled(devmap->device);
else
eer_flag = 0;
- return snprintf(buf, PAGE_SIZE, eer_flag ? "1\n" : "0\n");
+ return sysfs_emit(buf, eer_flag ? "1\n" : "0\n");
}

static ssize_t
@@ -1243,7 +1243,7 @@ dasd_expires_show(struct device *dev, struct device_attribute *attr, char *buf)
device = dasd_device_from_cdev(to_ccwdev(dev));
if (IS_ERR(device))
return -ENODEV;
- len = snprintf(buf, PAGE_SIZE, "%lu\n", device->default_expires);
+ len = sysfs_emit(buf, "%lu\n", device->default_expires);
dasd_put_device(device);
return len;
}
@@ -1283,7 +1283,7 @@ dasd_retries_show(struct device *dev, struct device_attribute *attr, char *buf)
device = dasd_device_from_cdev(to_ccwdev(dev));
if (IS_ERR(device))
return -ENODEV;
- len = snprintf(buf, PAGE_SIZE, "%lu\n", device->default_retries);
+ len = sysfs_emit(buf, "%lu\n", device->default_retries);
dasd_put_device(device);
return len;
}
@@ -1324,7 +1324,7 @@ dasd_timeout_show(struct device *dev, struct device_attribute *attr,
device = dasd_device_from_cdev(to_ccwdev(dev));
if (IS_ERR(device))
return -ENODEV;
- len = snprintf(buf, PAGE_SIZE, "%lu\n", device->blk_timeout);
+ len = sysfs_emit(buf, "%lu\n", device->blk_timeout);
dasd_put_device(device);
return len;
}
@@ -1398,11 +1398,11 @@ static ssize_t dasd_hpf_show(struct device *dev, struct device_attribute *attr,
return -ENODEV;
if (!device->discipline || !device->discipline->hpf_enabled) {
dasd_put_device(device);
- return snprintf(buf, PAGE_SIZE, "%d\n", dasd_nofcx);
+ return sysfs_emit(buf, "%d\n", dasd_nofcx);
}
hpf = device->discipline->hpf_enabled(device);
dasd_put_device(device);
- return snprintf(buf, PAGE_SIZE, "%d\n", hpf);
+ return sysfs_emit(buf, "%d\n", hpf);
}

static DEVICE_ATTR(hpf, 0444, dasd_hpf_show, NULL);
@@ -1416,13 +1416,13 @@ static ssize_t dasd_reservation_policy_show(struct device *dev,

devmap = dasd_find_busid(dev_name(dev));
if (IS_ERR(devmap)) {
- rc = snprintf(buf, PAGE_SIZE, "ignore\n");
+ rc = sysfs_emit(buf, "ignore\n");
} else {
spin_lock(&dasd_devmap_lock);
if (devmap->features & DASD_FEATURE_FAILONSLCK)
- rc = snprintf(buf, PAGE_SIZE, "fail\n");
+ rc = sysfs_emit(buf, "fail\n");
else
- rc = snprintf(buf, PAGE_SIZE, "ignore\n");
+ rc = sysfs_emit(buf, "ignore\n");
spin_unlock(&dasd_devmap_lock);
}
return rc;
@@ -1457,14 +1457,14 @@ static ssize_t dasd_reservation_state_show(struct device *dev,

device = dasd_device_from_cdev(to_ccwdev(dev));
if (IS_ERR(device))
- return snprintf(buf, PAGE_SIZE, "none\n");
+ return sysfs_emit(buf, "none\n");

if (test_bit(DASD_FLAG_IS_RESERVED, &device->flags))
- rc = snprintf(buf, PAGE_SIZE, "reserved\n");
+ rc = sysfs_emit(buf, "reserved\n");
else if (test_bit(DASD_FLAG_LOCK_STOLEN, &device->flags))
- rc = snprintf(buf, PAGE_SIZE, "lost\n");
+ rc = sysfs_emit(buf, "lost\n");
else
- rc = snprintf(buf, PAGE_SIZE, "none\n");
+ rc = sysfs_emit(buf, "none\n");
dasd_put_device(device);
return rc;
}
@@ -1531,7 +1531,7 @@ dasd_path_threshold_show(struct device *dev,
device = dasd_device_from_cdev(to_ccwdev(dev));
if (IS_ERR(device))
return -ENODEV;
- len = snprintf(buf, PAGE_SIZE, "%lu\n", device->path_thrhld);
+ len = sysfs_emit(buf, "%lu\n", device->path_thrhld);
dasd_put_device(device);
return len;
}
@@ -1578,7 +1578,7 @@ dasd_path_autodisable_show(struct device *dev,
else
flag = (DASD_FEATURE_DEFAULT &
DASD_FEATURE_PATH_AUTODISABLE) != 0;
- return snprintf(buf, PAGE_SIZE, flag ? "1\n" : "0\n");
+ return sysfs_emit(buf, flag ? "1\n" : "0\n");
}

static ssize_t
@@ -1616,7 +1616,7 @@ dasd_path_interval_show(struct device *dev,
device = dasd_device_from_cdev(to_ccwdev(dev));
if (IS_ERR(device))
return -ENODEV;
- len = snprintf(buf, PAGE_SIZE, "%lu\n", device->path_interval);
+ len = sysfs_emit(buf, "%lu\n", device->path_interval);
dasd_put_device(device);
return len;
}
@@ -1662,9 +1662,9 @@ dasd_device_fcs_show(struct device *dev, struct device_attribute *attr,
return -ENODEV;
fc_sec = dasd_path_get_fcs_device(device);
if (fc_sec == -EINVAL)
- rc = snprintf(buf, PAGE_SIZE, "Inconsistent\n");
+ rc = sysfs_emit(buf, "Inconsistent\n");
else
- rc = snprintf(buf, PAGE_SIZE, "%s\n", dasd_path_get_fcs_str(fc_sec));
+ rc = sysfs_emit(buf, "%s\n", dasd_path_get_fcs_str(fc_sec));
dasd_put_device(device);

return rc;
@@ -1677,7 +1677,7 @@ dasd_path_fcs_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
struct dasd_path *path = to_dasd_path(kobj);
unsigned int fc_sec = path->fc_security;

- return snprintf(buf, PAGE_SIZE, "%s\n", dasd_path_get_fcs_str(fc_sec));
+ return sysfs_emit(buf, "%s\n", dasd_path_get_fcs_str(fc_sec));
}

static struct kobj_attribute path_fcs_attribute =
@@ -1698,7 +1698,7 @@ static ssize_t dasd_##_name##_show(struct device *dev, \
val = _func(device); \
dasd_put_device(device); \
\
- return snprintf(buf, PAGE_SIZE, "%d\n", val); \
+ return sysfs_emit(buf, "%d\n", val); \
} \
static DEVICE_ATTR(_name, 0444, dasd_##_name##_show, NULL); \

diff --git a/drivers/s390/char/raw3270.c b/drivers/s390/char/raw3270.c
index 646ec796..be8529f 100644
--- a/drivers/s390/char/raw3270.c
+++ b/drivers/s390/char/raw3270.c
@@ -1047,7 +1047,7 @@ raw3270_probe (struct ccw_device *cdev)
static ssize_t
raw3270_model_show(struct device *dev, struct device_attribute *attr, char *buf)
{
- return snprintf(buf, PAGE_SIZE, "%i\n",
+ return sysfs_emit(buf, "%i\n",
((struct raw3270 *) dev_get_drvdata(dev))->model);
}
static DEVICE_ATTR(model, 0444, raw3270_model_show, NULL);
@@ -1055,7 +1055,7 @@ static DEVICE_ATTR(model, 0444, raw3270_model_show, NULL);
static ssize_t
raw3270_rows_show(struct device *dev, struct device_attribute *attr, char *buf)
{
- return snprintf(buf, PAGE_SIZE, "%i\n",
+ return sysfs_emit(buf, "%i\n",
((struct raw3270 *) dev_get_drvdata(dev))->rows);
}
static DEVICE_ATTR(rows, 0444, raw3270_rows_show, NULL);
@@ -1063,7 +1063,7 @@ static DEVICE_ATTR(rows, 0444, raw3270_rows_show, NULL);
static ssize_t
raw3270_columns_show(struct device *dev, struct device_attribute *attr, char *buf)
{
- return snprintf(buf, PAGE_SIZE, "%i\n",
+ return sysfs_emit(buf, "%i\n",
((struct raw3270 *) dev_get_drvdata(dev))->cols);
}
static DEVICE_ATTR(columns, 0444, raw3270_columns_show, NULL);
diff --git a/drivers/s390/cio/chp.c b/drivers/s390/cio/chp.c
index 1097e76..5440f28 100644
--- a/drivers/s390/cio/chp.c
+++ b/drivers/s390/cio/chp.c
@@ -285,7 +285,7 @@ static ssize_t chp_configure_show(struct device *dev,
if (status < 0)
return status;

- return snprintf(buf, PAGE_SIZE, "%d\n", status);
+ return sysfs_emit(buf, "%d\n", status);
}

static int cfg_wait_idle(void);
--
2.7.4

2021-10-18 10:53:57

by Heiko Carstens

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

On Thu, Oct 14, 2021 at 11:50:54PM -0700, Qing Wang 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.
>
> Signed-off-by: Qing Wang <[email protected]>
> ---
> drivers/s390/block/dasd_devmap.c | 74 ++++++++++++++++++++--------------------
> drivers/s390/char/raw3270.c | 6 ++--
> drivers/s390/cio/chp.c | 2 +-
> 3 files changed, 41 insertions(+), 41 deletions(-)

This requires either Reviewed-by or Acked-by from dasd and cio
maintainers, which you already cc'ed.

2021-10-18 11:52:54

by Vineeth Vijayan

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

Thanks.
Acked-by: Vineeth Vijayan <[email protected]>

(For the CIO part.)


On Thu, 2021-10-14 at 23:50 -0700, Qing Wang 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.
>
> Signed-off-by: Qing Wang <[email protected]>
> ---
> drivers/s390/block/dasd_devmap.c | 74 ++++++++++++++++++++--------
> ------------
> drivers/s390/char/raw3270.c | 6 ++--
> drivers/s390/cio/chp.c | 2 +-
> 3 files changed, 41 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/s390/block/dasd_devmap.c
> b/drivers/s390/block/dasd_devmap.c
> index 2c40fe1..78eb479 100644
> --- a/drivers/s390/block/dasd_devmap.c
> +++ b/drivers/s390/block/dasd_devmap.c
> @@ -731,7 +731,7 @@ static ssize_t dasd_ff_show(struct device *dev,
> struct device_attribute *attr,
> ff_flag = (devmap->features & DASD_FEATURE_FAILFAST) !=
> 0;
> else
> ff_flag = (DASD_FEATURE_DEFAULT &
> DASD_FEATURE_FAILFAST) != 0;
> - return snprintf(buf, PAGE_SIZE, ff_flag ? "1\n" : "0\n");
> + return sysfs_emit(buf, ff_flag ? "1\n" : "0\n");
> }
>
> static ssize_t dasd_ff_store(struct device *dev, struct
> device_attribute *attr,
> @@ -773,7 +773,7 @@ dasd_ro_show(struct device *dev, struct
> device_attribute *attr, char *buf)
> spin_unlock(&dasd_devmap_lock);
>
> out:
> - return snprintf(buf, PAGE_SIZE, ro_flag ? "1\n" : "0\n");
> + return sysfs_emit(buf, ro_flag ? "1\n" : "0\n");
> }
>
> static ssize_t
> @@ -834,7 +834,7 @@ dasd_erplog_show(struct device *dev, struct
> device_attribute *attr, char *buf)
> erplog = (devmap->features & DASD_FEATURE_ERPLOG) != 0;
> else
> erplog = (DASD_FEATURE_DEFAULT & DASD_FEATURE_ERPLOG)
> != 0;
> - return snprintf(buf, PAGE_SIZE, erplog ? "1\n" : "0\n");
> + return sysfs_emit(buf, erplog ? "1\n" : "0\n");
> }
>
> static ssize_t
> @@ -1033,13 +1033,13 @@ dasd_discipline_show(struct device *dev,
> struct device_attribute *attr,
> dasd_put_device(device);
> goto out;
> } else {
> - len = snprintf(buf, PAGE_SIZE, "%s\n",
> + len = sysfs_emit(buf, "%s\n",
> device->discipline->name);
> dasd_put_device(device);
> return len;
> }
> out:
> - len = snprintf(buf, PAGE_SIZE, "none\n");
> + len = sysfs_emit(buf, "none\n");
> return len;
> }
>
> @@ -1056,30 +1056,30 @@ dasd_device_status_show(struct device *dev,
> struct device_attribute *attr,
> if (!IS_ERR(device)) {
> switch (device->state) {
> case DASD_STATE_NEW:
> - len = snprintf(buf, PAGE_SIZE, "new\n");
> + len = sysfs_emit(buf, "new\n");
> break;
> case DASD_STATE_KNOWN:
> - len = snprintf(buf, PAGE_SIZE, "detected\n");
> + len = sysfs_emit(buf, "detected\n");
> break;
> case DASD_STATE_BASIC:
> - len = snprintf(buf, PAGE_SIZE, "basic\n");
> + len = sysfs_emit(buf, "basic\n");
> break;
> case DASD_STATE_UNFMT:
> - len = snprintf(buf, PAGE_SIZE,
> "unformatted\n");
> + len = sysfs_emit(buf, "unformatted\n");
> break;
> case DASD_STATE_READY:
> - len = snprintf(buf, PAGE_SIZE, "ready\n");
> + len = sysfs_emit(buf, "ready\n");
> break;
> case DASD_STATE_ONLINE:
> - len = snprintf(buf, PAGE_SIZE, "online\n");
> + len = sysfs_emit(buf, "online\n");
> break;
> default:
> - len = snprintf(buf, PAGE_SIZE, "no stat\n");
> + len = sysfs_emit(buf, "no stat\n");
> break;
> }
> dasd_put_device(device);
> } else
> - len = snprintf(buf, PAGE_SIZE, "unknown\n");
> + len = sysfs_emit(buf, "unknown\n");
> return len;
> }
>
> @@ -1120,7 +1120,7 @@ static ssize_t dasd_vendor_show(struct device
> *dev,
> device = dasd_device_from_cdev(to_ccwdev(dev));
> vendor = "";
> if (IS_ERR(device))
> - return snprintf(buf, PAGE_SIZE, "%s\n", vendor);
> + return sysfs_emit(buf, "%s\n", vendor);
>
> if (device->discipline && device->discipline->get_uid &&
> !device->discipline->get_uid(device, &uid))
> @@ -1128,7 +1128,7 @@ static ssize_t dasd_vendor_show(struct device
> *dev,
>
> dasd_put_device(device);
>
> - return snprintf(buf, PAGE_SIZE, "%s\n", vendor);
> + return sysfs_emit(buf, "%s\n", vendor);
> }
>
> static DEVICE_ATTR(vendor, 0444, dasd_vendor_show, NULL);
> @@ -1148,7 +1148,7 @@ dasd_uid_show(struct device *dev, struct
> device_attribute *attr, char *buf)
> device = dasd_device_from_cdev(to_ccwdev(dev));
> uid_string[0] = 0;
> if (IS_ERR(device))
> - return snprintf(buf, PAGE_SIZE, "%s\n", uid_string);
> + return sysfs_emit(buf, "%s\n", uid_string);
>
> if (device->discipline && device->discipline->get_uid &&
> !device->discipline->get_uid(device, &uid)) {
> @@ -1183,7 +1183,7 @@ dasd_uid_show(struct device *dev, struct
> device_attribute *attr, char *buf)
> }
> dasd_put_device(device);
>
> - return snprintf(buf, PAGE_SIZE, "%s\n", uid_string);
> + return sysfs_emit(buf, "%s\n", uid_string);
> }
> static DEVICE_ATTR(uid, 0444, dasd_uid_show, NULL);
>
> @@ -1201,7 +1201,7 @@ dasd_eer_show(struct device *dev, struct
> device_attribute *attr, char *buf)
> eer_flag = dasd_eer_enabled(devmap->device);
> else
> eer_flag = 0;
> - return snprintf(buf, PAGE_SIZE, eer_flag ? "1\n" : "0\n");
> + return sysfs_emit(buf, eer_flag ? "1\n" : "0\n");
> }
>
> static ssize_t
> @@ -1243,7 +1243,7 @@ dasd_expires_show(struct device *dev, struct
> device_attribute *attr, char *buf)
> device = dasd_device_from_cdev(to_ccwdev(dev));
> if (IS_ERR(device))
> return -ENODEV;
> - len = snprintf(buf, PAGE_SIZE, "%lu\n", device-
> >default_expires);
> + len = sysfs_emit(buf, "%lu\n", device->default_expires);
> dasd_put_device(device);
> return len;
> }
> @@ -1283,7 +1283,7 @@ dasd_retries_show(struct device *dev, struct
> device_attribute *attr, char *buf)
> device = dasd_device_from_cdev(to_ccwdev(dev));
> if (IS_ERR(device))
> return -ENODEV;
> - len = snprintf(buf, PAGE_SIZE, "%lu\n", device-
> >default_retries);
> + len = sysfs_emit(buf, "%lu\n", device->default_retries);
> dasd_put_device(device);
> return len;
> }
> @@ -1324,7 +1324,7 @@ dasd_timeout_show(struct device *dev, struct
> device_attribute *attr,
> device = dasd_device_from_cdev(to_ccwdev(dev));
> if (IS_ERR(device))
> return -ENODEV;
> - len = snprintf(buf, PAGE_SIZE, "%lu\n", device->blk_timeout);
> + len = sysfs_emit(buf, "%lu\n", device->blk_timeout);
> dasd_put_device(device);
> return len;
> }
> @@ -1398,11 +1398,11 @@ static ssize_t dasd_hpf_show(struct device
> *dev, struct device_attribute *attr,
> return -ENODEV;
> if (!device->discipline || !device->discipline->hpf_enabled) {
> dasd_put_device(device);
> - return snprintf(buf, PAGE_SIZE, "%d\n", dasd_nofcx);
> + return sysfs_emit(buf, "%d\n", dasd_nofcx);
> }
> hpf = device->discipline->hpf_enabled(device);
> dasd_put_device(device);
> - return snprintf(buf, PAGE_SIZE, "%d\n", hpf);
> + return sysfs_emit(buf, "%d\n", hpf);
> }
>
> static DEVICE_ATTR(hpf, 0444, dasd_hpf_show, NULL);
> @@ -1416,13 +1416,13 @@ static ssize_t
> dasd_reservation_policy_show(struct device *dev,
>
> devmap = dasd_find_busid(dev_name(dev));
> if (IS_ERR(devmap)) {
> - rc = snprintf(buf, PAGE_SIZE, "ignore\n");
> + rc = sysfs_emit(buf, "ignore\n");
> } else {
> spin_lock(&dasd_devmap_lock);
> if (devmap->features & DASD_FEATURE_FAILONSLCK)
> - rc = snprintf(buf, PAGE_SIZE, "fail\n");
> + rc = sysfs_emit(buf, "fail\n");
> else
> - rc = snprintf(buf, PAGE_SIZE, "ignore\n");
> + rc = sysfs_emit(buf, "ignore\n");
> spin_unlock(&dasd_devmap_lock);
> }
> return rc;
> @@ -1457,14 +1457,14 @@ static ssize_t
> dasd_reservation_state_show(struct device *dev,
>
> device = dasd_device_from_cdev(to_ccwdev(dev));
> if (IS_ERR(device))
> - return snprintf(buf, PAGE_SIZE, "none\n");
> + return sysfs_emit(buf, "none\n");
>
> if (test_bit(DASD_FLAG_IS_RESERVED, &device->flags))
> - rc = snprintf(buf, PAGE_SIZE, "reserved\n");
> + rc = sysfs_emit(buf, "reserved\n");
> else if (test_bit(DASD_FLAG_LOCK_STOLEN, &device->flags))
> - rc = snprintf(buf, PAGE_SIZE, "lost\n");
> + rc = sysfs_emit(buf, "lost\n");
> else
> - rc = snprintf(buf, PAGE_SIZE, "none\n");
> + rc = sysfs_emit(buf, "none\n");
> dasd_put_device(device);
> return rc;
> }
> @@ -1531,7 +1531,7 @@ dasd_path_threshold_show(struct device *dev,
> device = dasd_device_from_cdev(to_ccwdev(dev));
> if (IS_ERR(device))
> return -ENODEV;
> - len = snprintf(buf, PAGE_SIZE, "%lu\n", device->path_thrhld);
> + len = sysfs_emit(buf, "%lu\n", device->path_thrhld);
> dasd_put_device(device);
> return len;
> }
> @@ -1578,7 +1578,7 @@ dasd_path_autodisable_show(struct device *dev,
> else
> flag = (DASD_FEATURE_DEFAULT &
> DASD_FEATURE_PATH_AUTODISABLE) != 0;
> - return snprintf(buf, PAGE_SIZE, flag ? "1\n" : "0\n");
> + return sysfs_emit(buf, flag ? "1\n" : "0\n");
> }
>
> static ssize_t
> @@ -1616,7 +1616,7 @@ dasd_path_interval_show(struct device *dev,
> device = dasd_device_from_cdev(to_ccwdev(dev));
> if (IS_ERR(device))
> return -ENODEV;
> - len = snprintf(buf, PAGE_SIZE, "%lu\n", device->path_interval);
> + len = sysfs_emit(buf, "%lu\n", device->path_interval);
> dasd_put_device(device);
> return len;
> }
> @@ -1662,9 +1662,9 @@ dasd_device_fcs_show(struct device *dev, struct
> device_attribute *attr,
> return -ENODEV;
> fc_sec = dasd_path_get_fcs_device(device);
> if (fc_sec == -EINVAL)
> - rc = snprintf(buf, PAGE_SIZE, "Inconsistent\n");
> + rc = sysfs_emit(buf, "Inconsistent\n");
> else
> - rc = snprintf(buf, PAGE_SIZE, "%s\n",
> dasd_path_get_fcs_str(fc_sec));
> + rc = sysfs_emit(buf, "%s\n",
> dasd_path_get_fcs_str(fc_sec));
> dasd_put_device(device);
>
> return rc;
> @@ -1677,7 +1677,7 @@ dasd_path_fcs_show(struct kobject *kobj, struct
> kobj_attribute *attr, char *buf)
> struct dasd_path *path = to_dasd_path(kobj);
> unsigned int fc_sec = path->fc_security;
>
> - return snprintf(buf, PAGE_SIZE, "%s\n",
> dasd_path_get_fcs_str(fc_sec));
> + return sysfs_emit(buf, "%s\n", dasd_path_get_fcs_str(fc_sec));
> }
>
> static struct kobj_attribute path_fcs_attribute =
> @@ -1698,7 +1698,7 @@ static ssize_t dasd_##_name##_show(struct
> device *dev, \
> val = _func(device);
> \
> dasd_put_device(device); \
>
> \
> - return snprintf(buf, PAGE_SIZE, "%d\n", val);
> \
> + return sysfs_emit(buf, "%d\n", val); \
> }
> \
> static DEVICE_ATTR(_name, 0444, dasd_##_name##_show, NULL);
> \
>
> diff --git a/drivers/s390/char/raw3270.c
> b/drivers/s390/char/raw3270.c
> index 646ec796..be8529f 100644
> --- a/drivers/s390/char/raw3270.c
> +++ b/drivers/s390/char/raw3270.c
> @@ -1047,7 +1047,7 @@ raw3270_probe (struct ccw_device *cdev)
> static ssize_t
> raw3270_model_show(struct device *dev, struct device_attribute
> *attr, char *buf)
> {
> - return snprintf(buf, PAGE_SIZE, "%i\n",
> + return sysfs_emit(buf, "%i\n",
> ((struct raw3270 *) dev_get_drvdata(dev))-
> >model);
> }
> static DEVICE_ATTR(model, 0444, raw3270_model_show, NULL);
> @@ -1055,7 +1055,7 @@ static DEVICE_ATTR(model, 0444,
> raw3270_model_show, NULL);
> static ssize_t
> raw3270_rows_show(struct device *dev, struct device_attribute *attr,
> char *buf)
> {
> - return snprintf(buf, PAGE_SIZE, "%i\n",
> + return sysfs_emit(buf, "%i\n",
> ((struct raw3270 *) dev_get_drvdata(dev))-
> >rows);
> }
> static DEVICE_ATTR(rows, 0444, raw3270_rows_show, NULL);
> @@ -1063,7 +1063,7 @@ static DEVICE_ATTR(rows, 0444,
> raw3270_rows_show, NULL);
> static ssize_t
> raw3270_columns_show(struct device *dev, struct device_attribute
> *attr, char *buf)
> {
> - return snprintf(buf, PAGE_SIZE, "%i\n",
> + return sysfs_emit(buf, "%i\n",
> ((struct raw3270 *) dev_get_drvdata(dev))-
> >cols);
> }
> static DEVICE_ATTR(columns, 0444, raw3270_columns_show, NULL);
> diff --git a/drivers/s390/cio/chp.c b/drivers/s390/cio/chp.c
> index 1097e76..5440f28 100644
> --- a/drivers/s390/cio/chp.c
> +++ b/drivers/s390/cio/chp.c
> @@ -285,7 +285,7 @@ static ssize_t chp_configure_show(struct device
> *dev,
> if (status < 0)
> return status;
>
> - return snprintf(buf, PAGE_SIZE, "%d\n", status);
> + return sysfs_emit(buf, "%d\n", status);
> }
>
> static int cfg_wait_idle(void);

2021-10-18 18:24:43

by Joe Perches

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

On Mon, 2021-10-18 at 13:50 +0200, Vineeth Vijayan wrote:
> Acked-by: Vineeth Vijayan <[email protected]>
[]
> On Thu, 2021-10-14 at 23:50 -0700, Qing Wang wrote:
> > show() must not use snprintf() when formatting the value to be
> > returned to user space.
[]
> > diff --git a/drivers/s390/block/dasd_devmap.c b/drivers/s390/block/dasd_devmap.c
[]
> > @@ -731,7 +731,7 @@ static ssize_t dasd_ff_show(struct device *dev,
[]
> > - return snprintf(buf, PAGE_SIZE, ff_flag ? "1\n" : "0\n");
> > + return sysfs_emit(buf, ff_flag ? "1\n" : "0\n");

It's more common to use

return sysfs_emit(buf, "%d\n", ff_flag);

> > @@ -773,7 +773,7 @@ dasd_ro_show(struct device *dev, struct
[]
> > - return snprintf(buf, PAGE_SIZE, ro_flag ? "1\n" : "0\n");
> > + return sysfs_emit(buf, ro_flag ? "1\n" : "0\n");

etc...

2021-10-20 14:45:53

by Stefan Haberland

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

Am 15.10.21 um 08:50 schrieb Qing Wang:
> 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.
>
> Signed-off-by: Qing Wang <[email protected]>
> ---
> drivers/s390/block/dasd_devmap.c | 74 ++++++++++++++++++++--------------------
>

Thanks for the DASD changes. While I am totally fine with the change
itself I agree with Joe's remark.
Would you like to improve your patch? Otherwise I can add a small follow
on patch. Just let me know.

2021-11-10 15:21:49

by Heiko Carstens

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

On Wed, Oct 20, 2021 at 04:44:01PM +0200, Stefan Haberland wrote:
> Am 15.10.21 um 08:50 schrieb Qing Wang:
> > 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.
> >
> > Signed-off-by: Qing Wang <[email protected]>
> > ---
> > drivers/s390/block/dasd_devmap.c | 74 ++++++++++++++++++++--------------------
> >
>
> Thanks for the DASD changes. While I am totally fine with the change itself
> I agree with Joe's remark.
> Would you like to improve your patch? Otherwise I can add a small follow on
> patch. Just let me know.

I translated this to Acked-by. Please provide an addon patch if you
want to have this changed.
Applied, thanks!