Coccinnelle reports a warning
Warning: Use scnprintf or sprintf
Adding to that, there has been a slow migration from snprintf to scnprintf.
This LWN article explains the rationale for this change
https: //lwn.net/Articles/69419/
Ie. snprintf() returns what *would* be the resulting length,
while scnprintf() returns the actual length.
Signed-off-by: Jules Irenge <[email protected]>
---
drivers/s390/net/qeth_core_sys.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/s390/net/qeth_core_sys.c b/drivers/s390/net/qeth_core_sys.c
index 406be169173c..b40802d707a1 100644
--- a/drivers/s390/net/qeth_core_sys.c
+++ b/drivers/s390/net/qeth_core_sys.c
@@ -500,9 +500,9 @@ static ssize_t qeth_hw_trap_show(struct device *dev,
struct qeth_card *card = dev_get_drvdata(dev);
if (card->info.hwtrap)
- return snprintf(buf, 5, "arm\n");
+ return scnprintf(buf, 5, "arm\n");
else
- return snprintf(buf, 8, "disarm\n");
+ return scnprintf(buf, 8, "disarm\n");
}
static ssize_t qeth_hw_trap_store(struct device *dev,
--
2.37.3
On Mon, 26 Sep 2022 19:42:38 +0100 Jules Irenge wrote:
> Coccinnelle reports a warning
> Warning: Use scnprintf or sprintf
> Adding to that, there has been a slow migration from snprintf to scnprintf.
> This LWN article explains the rationale for this change
> https: //lwn.net/Articles/69419/
> Ie. snprintf() returns what *would* be the resulting length,
> while scnprintf() returns the actual length.
>
> Signed-off-by: Jules Irenge <[email protected]>
Looks legit but please repost this separately.
We only see patch 3 of the series.
On 27.09.22 02:33, Jakub Kicinski wrote:
> On Mon, 26 Sep 2022 19:42:38 +0100 Jules Irenge wrote:
>> Coccinnelle reports a warning
>> Warning: Use scnprintf or sprintf
>> Adding to that, there has been a slow migration from snprintf to scnprintf.
>> This LWN article explains the rationale for this change
>> https: //lwn.net/Articles/69419/
>> Ie. snprintf() returns what *would* be the resulting length,
>> while scnprintf() returns the actual length.
>>
>> Signed-off-by: Jules Irenge <[email protected]>
>
> Looks legit but please repost this separately.
> We only see patch 3 of the series.
When you repost, you can add
Reviewed-by: Alexandra Winter <[email protected]>
Thank you
Alexandra
On Mon, 2022-09-26 at 19:42 +0100, Jules Irenge wrote:
> Coccinnelle reports a warning
> Warning: Use scnprintf or sprintf
> Adding to that, there has been a slow migration from snprintf to scnprintf.
> This LWN article explains the rationale for this change
> https: //lwn.net/Articles/69419/
> Ie. snprintf() returns what *would* be the resulting length,
> while scnprintf() returns the actual length.
[]
> diff --git a/drivers/s390/net/qeth_core_sys.c b/drivers/s390/net/qeth_core_sys.c
[]
> @@ -500,9 +500,9 @@ static ssize_t qeth_hw_trap_show(struct device *dev,
> struct qeth_card *card = dev_get_drvdata(dev);
>
> if (card->info.hwtrap)
> - return snprintf(buf, 5, "arm\n");
> + return scnprintf(buf, 5, "arm\n");
> else
> - return snprintf(buf, 8, "disarm\n");
> + return scnprintf(buf, 8, "disarm\n");
> }
Use sysfs_emit instead.
For the entire file, perhaps something like: (untested)
---
drivers/s390/net/qeth_core_sys.c | 109 +++++++++++++++++++++------------------
1 file changed, 60 insertions(+), 49 deletions(-)
diff --git a/drivers/s390/net/qeth_core_sys.c b/drivers/s390/net/qeth_core_sys.c
index 406be169173ce..d7d6fd78129b3 100644
--- a/drivers/s390/net/qeth_core_sys.c
+++ b/drivers/s390/net/qeth_core_sys.c
@@ -20,19 +20,21 @@ static ssize_t qeth_dev_state_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct qeth_card *card = dev_get_drvdata(dev);
+ const char *type = "UNKNOWN";
switch (card->state) {
case CARD_STATE_DOWN:
- return sprintf(buf, "DOWN\n");
+ type = "DOWN";
+ break;
case CARD_STATE_SOFTSETUP:
- if (card->dev->flags & IFF_UP)
- return sprintf(buf, "UP (LAN %s)\n",
- netif_carrier_ok(card->dev) ? "ONLINE" :
- "OFFLINE");
- return sprintf(buf, "SOFTSETUP\n");
- default:
- return sprintf(buf, "UNKNOWN\n");
+ if (!(card->dev->flags & IFF_UP)) {
+ type = "SOFTSETUP";
+ break;
+ }
+ return sysfs_emit(buf, "UP (LAN %sLINE)\n",
+ netif_carrier_ok(card->dev) ? "ON" : "OFF");
}
+ return sysfs_emit(buf, "%s\n", type);
}
static DEVICE_ATTR(state, 0444, qeth_dev_state_show, NULL);
@@ -42,7 +44,7 @@ static ssize_t qeth_dev_chpid_show(struct device *dev,
{
struct qeth_card *card = dev_get_drvdata(dev);
- return sprintf(buf, "%02X\n", card->info.chpid);
+ return sysfs_emit(buf, "%02X\n", card->info.chpid);
}
static DEVICE_ATTR(chpid, 0444, qeth_dev_chpid_show, NULL);
@@ -52,7 +54,7 @@ static ssize_t qeth_dev_if_name_show(struct device *dev,
{
struct qeth_card *card = dev_get_drvdata(dev);
- return sprintf(buf, "%s\n", netdev_name(card->dev));
+ return sysfs_emit(buf, "%s\n", netdev_name(card->dev));
}
static DEVICE_ATTR(if_name, 0444, qeth_dev_if_name_show, NULL);
@@ -62,7 +64,7 @@ static ssize_t qeth_dev_card_type_show(struct device *dev,
{
struct qeth_card *card = dev_get_drvdata(dev);
- return sprintf(buf, "%s\n", qeth_get_cardname_short(card));
+ return sysfs_emit(buf, "%s\n", qeth_get_cardname_short(card));
}
static DEVICE_ATTR(card_type, 0444, qeth_dev_card_type_show, NULL);
@@ -86,7 +88,7 @@ static ssize_t qeth_dev_inbuf_size_show(struct device *dev,
{
struct qeth_card *card = dev_get_drvdata(dev);
- return sprintf(buf, "%s\n", qeth_get_bufsize_str(card));
+ return sysfs_emit(buf, "%s\n", qeth_get_bufsize_str(card));
}
static DEVICE_ATTR(inbuf_size, 0444, qeth_dev_inbuf_size_show, NULL);
@@ -96,7 +98,7 @@ static ssize_t qeth_dev_portno_show(struct device *dev,
{
struct qeth_card *card = dev_get_drvdata(dev);
- return sprintf(buf, "%i\n", card->dev->dev_port);
+ return sysfs_emit(buf, "%i\n", card->dev->dev_port);
}
static ssize_t qeth_dev_portno_store(struct device *dev,
@@ -134,7 +136,7 @@ static DEVICE_ATTR(portno, 0644, qeth_dev_portno_show, qeth_dev_portno_store);
static ssize_t qeth_dev_portname_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
- return sprintf(buf, "no portname required\n");
+ return sysfs_emit(buf, "no portname required\n");
}
static ssize_t qeth_dev_portname_store(struct device *dev,
@@ -154,22 +156,27 @@ static ssize_t qeth_dev_prioqing_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct qeth_card *card = dev_get_drvdata(dev);
+ const char *type = "disabled";
switch (card->qdio.do_prio_queueing) {
case QETH_PRIO_Q_ING_PREC:
- return sprintf(buf, "%s\n", "by precedence");
+ type = "by precedence";
+ break;
case QETH_PRIO_Q_ING_TOS:
- return sprintf(buf, "%s\n", "by type of service");
+ type = "by type of service";
+ break;
case QETH_PRIO_Q_ING_SKB:
- return sprintf(buf, "%s\n", "by skb-priority");
+ type = "by skb-priority";
+ break;
case QETH_PRIO_Q_ING_VLAN:
- return sprintf(buf, "%s\n", "by VLAN headers");
+ type = "by VLAN headers";
+ break;
case QETH_PRIO_Q_ING_FIXED:
- return sprintf(buf, "always queue %i\n",
- card->qdio.default_out_queue);
- default:
- return sprintf(buf, "disabled\n");
+ return sysfs_emit(buf, "always queue %i\n",
+ card->qdio.default_out_queue);
}
+
+ return sysfs_emit(buf, "%s\n", type);
}
static ssize_t qeth_dev_prioqing_store(struct device *dev,
@@ -242,7 +249,7 @@ static ssize_t qeth_dev_bufcnt_show(struct device *dev,
{
struct qeth_card *card = dev_get_drvdata(dev);
- return sprintf(buf, "%i\n", card->qdio.in_buf_pool.buf_count);
+ return sysfs_emit(buf, "%i\n", card->qdio.in_buf_pool.buf_count);
}
static ssize_t qeth_dev_bufcnt_store(struct device *dev,
@@ -298,7 +305,7 @@ static DEVICE_ATTR(recover, 0200, NULL, qeth_dev_recover_store);
static ssize_t qeth_dev_performance_stats_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
- return sprintf(buf, "1\n");
+ return sysfs_emit(buf, "1\n");
}
static ssize_t qeth_dev_performance_stats_store(struct device *dev,
@@ -335,7 +342,7 @@ static ssize_t qeth_dev_layer2_show(struct device *dev,
{
struct qeth_card *card = dev_get_drvdata(dev);
- return sprintf(buf, "%i\n", card->options.layer);
+ return sysfs_emit(buf, "%i\n", card->options.layer);
}
static ssize_t qeth_dev_layer2_store(struct device *dev,
@@ -407,17 +414,21 @@ static ssize_t qeth_dev_isolation_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct qeth_card *card = dev_get_drvdata(dev);
+ const char *type = "N/A";
switch (card->options.isolation) {
case ISOLATION_MODE_NONE:
- return snprintf(buf, 6, "%s\n", ATTR_QETH_ISOLATION_NONE);
+ type = ATTR_QETH_ISOLATION_NONE;
+ break;
case ISOLATION_MODE_FWD:
- return snprintf(buf, 9, "%s\n", ATTR_QETH_ISOLATION_FWD);
+ type = ATTR_QETH_ISOLATION_FWD;
+ break;
case ISOLATION_MODE_DROP:
- return snprintf(buf, 6, "%s\n", ATTR_QETH_ISOLATION_DROP);
- default:
- return snprintf(buf, 5, "%s\n", "N/A");
+ type = ATTR_QETH_ISOLATION_DROP;
+ break;
}
+
+ return sysfs_emit("%s\n", type);
}
static ssize_t qeth_dev_isolation_store(struct device *dev,
@@ -467,28 +478,31 @@ static ssize_t qeth_dev_switch_attrs_show(struct device *dev,
{
struct qeth_card *card = dev_get_drvdata(dev);
struct qeth_switch_info sw_info;
- int rc = 0;
+ int len = 0;
+ int rc;
if (!qeth_card_hw_is_reachable(card))
- return sprintf(buf, "n/a\n");
+ return sysfs_emit(buf, "n/a\n");
rc = qeth_query_switch_attributes(card, &sw_info);
if (rc)
return rc;
if (!sw_info.capabilities)
- rc = sprintf(buf, "unknown");
+ return sysfs_emit(buf, "unknown\n");
if (sw_info.capabilities & QETH_SWITCH_FORW_802_1)
- rc = sprintf(buf, (sw_info.settings & QETH_SWITCH_FORW_802_1 ?
- "[802.1]" : "802.1"));
- if (sw_info.capabilities & QETH_SWITCH_FORW_REFL_RELAY)
- rc += sprintf(buf + rc,
- (sw_info.settings & QETH_SWITCH_FORW_REFL_RELAY ?
- " [rr]" : " rr"));
- rc += sprintf(buf + rc, "\n");
-
- return rc;
+ len += sysfs_emit_at(buf, len,
+ sw_info.settings & QETH_SWITCH_FORW_802_1 ?
+ "[802.1]" : "802.1");
+ if (sw_info.capabilities & QETH_SWITCH_FORW_REFL_RELAY) {
+ if (len)
+ len += sysfs_emit_at(buf, len, " ");
+ len += sysfs_emit_at(buf, len,
+ sw_info.settings & QETH_SWITCH_FORW_REFL_RELAY ?
+ "[rr]" : "rr");
+ }
+ return sysfs_emit_at(buf, len, "\n");
}
static DEVICE_ATTR(switch_attrs, 0444,
@@ -499,10 +513,7 @@ static ssize_t qeth_hw_trap_show(struct device *dev,
{
struct qeth_card *card = dev_get_drvdata(dev);
- if (card->info.hwtrap)
- return snprintf(buf, 5, "arm\n");
- else
- return snprintf(buf, 8, "disarm\n");
+ return sysfs_emit(buf, "%s\n", card->info.hwtrap ? "arm" : "disarm");
}
static ssize_t qeth_hw_trap_store(struct device *dev,
@@ -573,7 +584,7 @@ static ssize_t qeth_dev_blkt_total_show(struct device *dev,
{
struct qeth_card *card = dev_get_drvdata(dev);
- return sprintf(buf, "%i\n", card->info.blkt.time_total);
+ return sysfs_emit(buf, "%i\n", card->info.blkt.time_total);
}
static ssize_t qeth_dev_blkt_total_store(struct device *dev,
@@ -593,7 +604,7 @@ static ssize_t qeth_dev_blkt_inter_show(struct device *dev,
{
struct qeth_card *card = dev_get_drvdata(dev);
- return sprintf(buf, "%i\n", card->info.blkt.inter_packet);
+ return sysfs_emit(buf, "%i\n", card->info.blkt.inter_packet);
}
static ssize_t qeth_dev_blkt_inter_store(struct device *dev,
@@ -613,7 +624,7 @@ static ssize_t qeth_dev_blkt_inter_jumbo_show(struct device *dev,
{
struct qeth_card *card = dev_get_drvdata(dev);
- return sprintf(buf, "%i\n", card->info.blkt.inter_packet_jumbo);
+ return sysfs_emit(buf, "%i\n", card->info.blkt.inter_packet_jumbo);
}
static ssize_t qeth_dev_blkt_inter_jumbo_store(struct device *dev,
On 27.09.22 16:27, Joe Perches wrote:
> On Mon, 2022-09-26 at 19:42 +0100, Jules Irenge wrote:
>> Coccinnelle reports a warning
>> Warning: Use scnprintf or sprintf
>> Adding to that, there has been a slow migration from snprintf to scnprintf.
>> This LWN article explains the rationale for this change
>> https: //lwn.net/Articles/69419/
>> Ie. snprintf() returns what *would* be the resulting length,
>> while scnprintf() returns the actual length.
> []
>> diff --git a/drivers/s390/net/qeth_core_sys.c b/drivers/s390/net/qeth_core_sys.c
> []
>> @@ -500,9 +500,9 @@ static ssize_t qeth_hw_trap_show(struct device *dev,
>> struct qeth_card *card = dev_get_drvdata(dev);
>>
>> if (card->info.hwtrap)
>> - return snprintf(buf, 5, "arm\n");
>> + return scnprintf(buf, 5, "arm\n");
>> else
>> - return snprintf(buf, 8, "disarm\n");
>> + return scnprintf(buf, 8, "disarm\n");
>> }
>
> Use sysfs_emit instead.
>
Thank you Joe, that sounds like the best way to handle this.
I propose that I take this onto my ToDo list and test it in the s390 environment.
I will add
Reported-by: Jules Irenge <[email protected]>
Suggested-by: Joe Perches <[email protected]>
On Wed, 2022-09-28 at 10:24 +0200, Alexandra Winter wrote:
> On 27.09.22 16:27, Joe Perches wrote:
> > On Mon, 2022-09-26 at 19:42 +0100, Jules Irenge wrote:
> > > Coccinnelle reports a warning
> > > Warning: Use scnprintf or sprintf
> > > Adding to that, there has been a slow migration from snprintf to scnprintf.
> > > This LWN article explains the rationale for this change
> > > https: //lwn.net/Articles/69419/
> > > Ie. snprintf() returns what *would* be the resulting length,
> > > while scnprintf() returns the actual length.
> > []
> > > diff --git a/drivers/s390/net/qeth_core_sys.c b/drivers/s390/net/qeth_core_sys.c
> > []
> > > @@ -500,9 +500,9 @@ static ssize_t qeth_hw_trap_show(struct device *dev,
> > > struct qeth_card *card = dev_get_drvdata(dev);
> > >
> > > if (card->info.hwtrap)
> > > - return snprintf(buf, 5, "arm\n");
> > > + return scnprintf(buf, 5, "arm\n");
> > > else
> > > - return snprintf(buf, 8, "disarm\n");
> > > + return scnprintf(buf, 8, "disarm\n");
> > > }
> >
> > Use sysfs_emit instead.
> >
>
> Thank you Joe, that sounds like the best way to handle this.
> I propose that I take this onto my ToDo list and test it in the s390 environment.
> I will add
> Reported-by: Jules Irenge <[email protected]>
> Suggested-by: Joe Perches <[email protected]>
>
Thanks.
btw: I was careless when I wrote one section of the proposed patch.
In this patch block,
@@ -467,28 +478,31 @@ static ssize_t qeth_dev_switch_attrs_show(struct device *dev,
The last line
+ return sysfs_emit_at(buf, len, "\n");
is not correct
It needs to be changed to:
len += sysfs_emit_at(buf, len, "\n");
return len;