diff mbox series

[net-next,v2,4/4] s390/ctcm: Convert sprintf/snprintf to scnprintf

Message ID 20230621134921.904217-5-wintera@linux.ibm.com (mailing list archive)
State Accepted
Commit 1471d85ffba7d17456670afa1b75e50234caa2c6
Delegated to: Netdev Maintainers
Headers show
Series s390/net: updates 2023-06-10 | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 8 this patch: 8
netdev/cc_maintainers warning 5 maintainers not CCed: borntraeger@linux.ibm.com gor@linux.ibm.com wenjia@linux.ibm.com agordeev@linux.ibm.com svens@linux.ibm.com
netdev/build_clang success Errors and warnings before: 8 this patch: 8
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 8 this patch: 8
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 130 lines checked
netdev/kdoc success Errors and warnings before: 2 this patch: 2
netdev/source_inline success Was 0 now: 0

Commit Message

Alexandra Winter June 21, 2023, 1:49 p.m. UTC 
From: Thorsten Winkler <twinkler@linux.ibm.com>

This LWN article explains the why scnprintf is preferred over snprintf
in general
https://lwn.net/Articles/69419/
Ie. snprintf() returns what *would* be the resulting length, while
scnprintf() returns the actual length.

Note that ctcm_print_statistics() writes the data into the kernel log
and is therefore not suitable for sysfs_emit(). Observable behavior is
not changed, as there may be dependencies.

Reviewed-by: Alexandra Winter <wintera@linux.ibm.com>
Signed-off-by: Thorsten Winkler <twinkler@linux.ibm.com>
Signed-off-by: Alexandra Winter <wintera@linux.ibm.com>
---
 drivers/s390/net/ctcm_dbug.c  |  2 +-
 drivers/s390/net/ctcm_main.c  |  6 +++---
 drivers/s390/net/ctcm_main.h  |  1 +
 drivers/s390/net/ctcm_mpc.c   | 18 ++++++++++--------
 drivers/s390/net/ctcm_sysfs.c | 36 +++++++++++++++++------------------
 5 files changed, 33 insertions(+), 30 deletions(-)

Comments

Paolo Abeni June 23, 2023, 9:55 a.m. UTC | #1 
On Wed, 2023-06-21 at 15:49 +0200, Alexandra Winter wrote:
> From: Thorsten Winkler <twinkler@linux.ibm.com>
> 
> This LWN article explains the why scnprintf is preferred over snprintf
> in general
> https://lwn.net/Articles/69419/
> Ie. snprintf() returns what *would* be the resulting length, while
> scnprintf() returns the actual length.
> 
> Note that ctcm_print_statistics() writes the data into the kernel log
> and is therefore not suitable for sysfs_emit(). Observable behavior is
> not changed, as there may be dependencies.
> 
> Reviewed-by: Alexandra Winter <wintera@linux.ibm.com>
> Signed-off-by: Thorsten Winkler <twinkler@linux.ibm.com>
> Signed-off-by: Alexandra Winter <wintera@linux.ibm.com>
> ---
>  drivers/s390/net/ctcm_dbug.c  |  2 +-
>  drivers/s390/net/ctcm_main.c  |  6 +++---
>  drivers/s390/net/ctcm_main.h  |  1 +
>  drivers/s390/net/ctcm_mpc.c   | 18 ++++++++++--------
>  drivers/s390/net/ctcm_sysfs.c | 36 +++++++++++++++++------------------
>  5 files changed, 33 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/s390/net/ctcm_dbug.c b/drivers/s390/net/ctcm_dbug.c
> index f7ec51db3cd6..b6f0e2f114b4 100644
> --- a/drivers/s390/net/ctcm_dbug.c
> +++ b/drivers/s390/net/ctcm_dbug.c
> @@ -70,7 +70,7 @@ void ctcm_dbf_longtext(enum ctcm_dbf_names dbf_nix, int level, char *fmt, ...)
>  	if (!debug_level_enabled(ctcm_dbf[dbf_nix].id, level))
>  		return;
>  	va_start(args, fmt);
> -	vsnprintf(dbf_txt_buf, sizeof(dbf_txt_buf), fmt, args);
> +	vscnprintf(dbf_txt_buf, sizeof(dbf_txt_buf), fmt, args);
>  	va_end(args);
>  
>  	debug_text_event(ctcm_dbf[dbf_nix].id, level, dbf_txt_buf);
> diff --git a/drivers/s390/net/ctcm_main.c b/drivers/s390/net/ctcm_main.c
> index e0fdd54bfeb7..79fac5314e67 100644
> --- a/drivers/s390/net/ctcm_main.c
> +++ b/drivers/s390/net/ctcm_main.c
> @@ -1340,7 +1340,7 @@ static int add_channel(struct ccw_device *cdev, enum ctcm_channel_types type,
>  					goto nomem_return;
>  
>  	ch->cdev = cdev;
> -	snprintf(ch->id, CTCM_ID_SIZE, "ch-%s", dev_name(&cdev->dev));
> +	scnprintf(ch->id, CTCM_ID_SIZE, "ch-%s", dev_name(&cdev->dev));
>  	ch->type = type;
>  
>  	/*
> @@ -1505,8 +1505,8 @@ static int ctcm_new_device(struct ccwgroup_device *cgdev)
>  
>  	type = get_channel_type(&cdev0->id);
>  
> -	snprintf(read_id, CTCM_ID_SIZE, "ch-%s", dev_name(&cdev0->dev));
> -	snprintf(write_id, CTCM_ID_SIZE, "ch-%s", dev_name(&cdev1->dev));
> +	scnprintf(read_id, CTCM_ID_SIZE, "ch-%s", dev_name(&cdev0->dev));
> +	scnprintf(write_id, CTCM_ID_SIZE, "ch-%s", dev_name(&cdev1->dev));
>  
>  	ret = add_channel(cdev0, type, priv);
>  	if (ret) {
> diff --git a/drivers/s390/net/ctcm_main.h b/drivers/s390/net/ctcm_main.h
> index 90bd7b3f80c3..25164e8bf13d 100644
> --- a/drivers/s390/net/ctcm_main.h
> +++ b/drivers/s390/net/ctcm_main.h
> @@ -100,6 +100,7 @@ enum ctcm_channel_types {
>  #define CTCM_PROTO_MPC		4
>  #define CTCM_PROTO_MAX		4
>  
> +#define CTCM_STATSIZE_LIMIT	64
>  #define CTCM_BUFSIZE_LIMIT	65535
>  #define CTCM_BUFSIZE_DEFAULT	32768
>  #define MPC_BUFSIZE_DEFAULT	CTCM_BUFSIZE_LIMIT
> diff --git a/drivers/s390/net/ctcm_mpc.c b/drivers/s390/net/ctcm_mpc.c
> index 8ac213a55141..64996c86defc 100644
> --- a/drivers/s390/net/ctcm_mpc.c
> +++ b/drivers/s390/net/ctcm_mpc.c
> @@ -144,9 +144,9 @@ void ctcmpc_dumpit(char *buf, int len)
>  
>  	for (ct = 0; ct < len; ct++, ptr++, rptr++) {
>  		if (sw == 0) {
> -			sprintf(addr, "%16.16llx", (__u64)rptr);
> +			scnprintf(addr, sizeof(addr), "%16.16llx", (__u64)rptr);
>  
> -			sprintf(boff, "%4.4X", (__u32)ct);
> +			scnprintf(boff, sizeof(boff), "%4.4X", (__u32)ct);
>  			bhex[0] = '\0';
>  			basc[0] = '\0';
>  		}
> @@ -155,7 +155,7 @@ void ctcmpc_dumpit(char *buf, int len)
>  		if (sw == 8)
>  			strcat(bhex, "	");
>  
> -		sprintf(tbuf, "%2.2llX", (__u64)*ptr);
> +		scnprintf(tbuf, sizeof(tbuf), "%2.2llX", (__u64)*ptr);
>  
>  		tbuf[2] = '\0';
>  		strcat(bhex, tbuf);
> @@ -171,8 +171,8 @@ void ctcmpc_dumpit(char *buf, int len)
>  			continue;
>  		if ((strcmp(duphex, bhex)) != 0) {
>  			if (dup != 0) {
> -				sprintf(tdup,
> -					"Duplicate as above to %s", addr);
> +				scnprintf(tdup, sizeof(tdup),
> +					  "Duplicate as above to %s", addr);
>  				ctcm_pr_debug("		       --- %s ---\n",
>  						tdup);
>  			}
> @@ -197,14 +197,16 @@ void ctcmpc_dumpit(char *buf, int len)
>  			strcat(basc, " ");
>  		}
>  		if (dup != 0) {
> -			sprintf(tdup, "Duplicate as above to %s", addr);
> +			scnprintf(tdup, sizeof(tdup),
> +				  "Duplicate as above to %s", addr);
>  			ctcm_pr_debug("		       --- %s ---\n", tdup);
>  		}
>  		ctcm_pr_debug("   %s (+%s) : %s  [%s]\n",
>  					addr, boff, bhex, basc);
>  	} else {
>  		if (dup >= 1) {
> -			sprintf(tdup, "Duplicate as above to %s", addr);
> +			scnprintf(tdup, sizeof(tdup),
> +				  "Duplicate as above to %s", addr);
>  			ctcm_pr_debug("		       --- %s ---\n", tdup);
>  		}
>  		if (dup != 0) {
> @@ -291,7 +293,7 @@ static struct net_device *ctcmpc_get_dev(int port_num)
>  	struct net_device *dev;
>  	struct ctcm_priv *priv;
>  
> -	sprintf(device, "%s%i", MPC_DEVICE_NAME, port_num);
> +	scnprintf(device, sizeof(device), "%s%i", MPC_DEVICE_NAME, port_num);
>  
>  	dev = __dev_get_by_name(&init_net, device);
>  
> diff --git a/drivers/s390/net/ctcm_sysfs.c b/drivers/s390/net/ctcm_sysfs.c
> index 98680c2cc4e4..0c5d8a3eaa2e 100644
> --- a/drivers/s390/net/ctcm_sysfs.c
> +++ b/drivers/s390/net/ctcm_sysfs.c
> @@ -86,24 +86,24 @@ static void ctcm_print_statistics(struct ctcm_priv *priv)
>  		return;
>  	p = sbuf;
>  
> -	p += sprintf(p, "  Device FSM state: %s\n",
> -		     fsm_getstate_str(priv->fsm));
> -	p += sprintf(p, "  RX channel FSM state: %s\n",
> -		     fsm_getstate_str(priv->channel[CTCM_READ]->fsm));
> -	p += sprintf(p, "  TX channel FSM state: %s\n",
> -		     fsm_getstate_str(priv->channel[CTCM_WRITE]->fsm));
> -	p += sprintf(p, "  Max. TX buffer used: %ld\n",
> -		     priv->channel[WRITE]->prof.maxmulti);
> -	p += sprintf(p, "  Max. chained SKBs: %ld\n",
> -		     priv->channel[WRITE]->prof.maxcqueue);
> -	p += sprintf(p, "  TX single write ops: %ld\n",
> -		     priv->channel[WRITE]->prof.doios_single);
> -	p += sprintf(p, "  TX multi write ops: %ld\n",
> -		     priv->channel[WRITE]->prof.doios_multi);
> -	p += sprintf(p, "  Netto bytes written: %ld\n",
> -		     priv->channel[WRITE]->prof.txlen);
> -	p += sprintf(p, "  Max. TX IO-time: %u\n",
> -		     jiffies_to_usecs(priv->channel[WRITE]->prof.tx_time));
> +	p += scnprintf(p, CTCM_STATSIZE_LIMIT, "  Device FSM state: %s\n",
> +		       fsm_getstate_str(priv->fsm));
> +	p += scnprintf(p, CTCM_STATSIZE_LIMIT, "  RX channel FSM state: %s\n",
> +		       fsm_getstate_str(priv->channel[CTCM_READ]->fsm));
> +	p += scnprintf(p, CTCM_STATSIZE_LIMIT, "  TX channel FSM state: %s\n",
> +		       fsm_getstate_str(priv->channel[CTCM_WRITE]->fsm));
> +	p += scnprintf(p, CTCM_STATSIZE_LIMIT, "  Max. TX buffer used: %ld\n",
> +		       priv->channel[WRITE]->prof.maxmulti);
> +	p += scnprintf(p, CTCM_STATSIZE_LIMIT, "  Max. chained SKBs: %ld\n",
> +		       priv->channel[WRITE]->prof.maxcqueue);
> +	p += scnprintf(p, CTCM_STATSIZE_LIMIT, "  TX single write ops: %ld\n",
> +		       priv->channel[WRITE]->prof.doios_single);
> +	p += scnprintf(p, CTCM_STATSIZE_LIMIT, "  TX multi write ops: %ld\n",
> +		       priv->channel[WRITE]->prof.doios_multi);
> +	p += scnprintf(p, CTCM_STATSIZE_LIMIT, "  Netto bytes written: %ld\n",
> +		       priv->channel[WRITE]->prof.txlen);
> +	p += scnprintf(p, CTCM_STATSIZE_LIMIT, "  Max. TX IO-time: %u\n",
> +		       jiffies_to_usecs(priv->channel[WRITE]->prof.tx_time));
>  
>  	printk(KERN_INFO "Statistics for %s:\n%s",
>  				priv->channel[CTCM_WRITE]->netdev->name, sbuf);

Not strictly related to this series, but it looks like there is
opportunity for more cleanups? e.g. move legacy printk to pr_info(),
etc...

The series LGTM.

Thanks,

Paolo
diff mbox series

Patch

diff --git a/drivers/s390/net/ctcm_dbug.c b/drivers/s390/net/ctcm_dbug.c
index f7ec51db3cd6..b6f0e2f114b4 100644
--- a/drivers/s390/net/ctcm_dbug.c
+++ b/drivers/s390/net/ctcm_dbug.c
@@ -70,7 +70,7 @@  void ctcm_dbf_longtext(enum ctcm_dbf_names dbf_nix, int level, char *fmt, ...)
 	if (!debug_level_enabled(ctcm_dbf[dbf_nix].id, level))
 		return;
 	va_start(args, fmt);
-	vsnprintf(dbf_txt_buf, sizeof(dbf_txt_buf), fmt, args);
+	vscnprintf(dbf_txt_buf, sizeof(dbf_txt_buf), fmt, args);
 	va_end(args);
 
 	debug_text_event(ctcm_dbf[dbf_nix].id, level, dbf_txt_buf);
diff --git a/drivers/s390/net/ctcm_main.c b/drivers/s390/net/ctcm_main.c
index e0fdd54bfeb7..79fac5314e67 100644
--- a/drivers/s390/net/ctcm_main.c
+++ b/drivers/s390/net/ctcm_main.c
@@ -1340,7 +1340,7 @@  static int add_channel(struct ccw_device *cdev, enum ctcm_channel_types type,
 					goto nomem_return;
 
 	ch->cdev = cdev;
-	snprintf(ch->id, CTCM_ID_SIZE, "ch-%s", dev_name(&cdev->dev));
+	scnprintf(ch->id, CTCM_ID_SIZE, "ch-%s", dev_name(&cdev->dev));
 	ch->type = type;
 
 	/*
@@ -1505,8 +1505,8 @@  static int ctcm_new_device(struct ccwgroup_device *cgdev)
 
 	type = get_channel_type(&cdev0->id);
 
-	snprintf(read_id, CTCM_ID_SIZE, "ch-%s", dev_name(&cdev0->dev));
-	snprintf(write_id, CTCM_ID_SIZE, "ch-%s", dev_name(&cdev1->dev));
+	scnprintf(read_id, CTCM_ID_SIZE, "ch-%s", dev_name(&cdev0->dev));
+	scnprintf(write_id, CTCM_ID_SIZE, "ch-%s", dev_name(&cdev1->dev));
 
 	ret = add_channel(cdev0, type, priv);
 	if (ret) {
diff --git a/drivers/s390/net/ctcm_main.h b/drivers/s390/net/ctcm_main.h
index 90bd7b3f80c3..25164e8bf13d 100644
--- a/drivers/s390/net/ctcm_main.h
+++ b/drivers/s390/net/ctcm_main.h
@@ -100,6 +100,7 @@  enum ctcm_channel_types {
 #define CTCM_PROTO_MPC		4
 #define CTCM_PROTO_MAX		4
 
+#define CTCM_STATSIZE_LIMIT	64
 #define CTCM_BUFSIZE_LIMIT	65535
 #define CTCM_BUFSIZE_DEFAULT	32768
 #define MPC_BUFSIZE_DEFAULT	CTCM_BUFSIZE_LIMIT
diff --git a/drivers/s390/net/ctcm_mpc.c b/drivers/s390/net/ctcm_mpc.c
index 8ac213a55141..64996c86defc 100644
--- a/drivers/s390/net/ctcm_mpc.c
+++ b/drivers/s390/net/ctcm_mpc.c
@@ -144,9 +144,9 @@  void ctcmpc_dumpit(char *buf, int len)
 
 	for (ct = 0; ct < len; ct++, ptr++, rptr++) {
 		if (sw == 0) {
-			sprintf(addr, "%16.16llx", (__u64)rptr);
+			scnprintf(addr, sizeof(addr), "%16.16llx", (__u64)rptr);
 
-			sprintf(boff, "%4.4X", (__u32)ct);
+			scnprintf(boff, sizeof(boff), "%4.4X", (__u32)ct);
 			bhex[0] = '\0';
 			basc[0] = '\0';
 		}
@@ -155,7 +155,7 @@  void ctcmpc_dumpit(char *buf, int len)
 		if (sw == 8)
 			strcat(bhex, "	");
 
-		sprintf(tbuf, "%2.2llX", (__u64)*ptr);
+		scnprintf(tbuf, sizeof(tbuf), "%2.2llX", (__u64)*ptr);
 
 		tbuf[2] = '\0';
 		strcat(bhex, tbuf);
@@ -171,8 +171,8 @@  void ctcmpc_dumpit(char *buf, int len)
 			continue;
 		if ((strcmp(duphex, bhex)) != 0) {
 			if (dup != 0) {
-				sprintf(tdup,
-					"Duplicate as above to %s", addr);
+				scnprintf(tdup, sizeof(tdup),
+					  "Duplicate as above to %s", addr);
 				ctcm_pr_debug("		       --- %s ---\n",
 						tdup);
 			}
@@ -197,14 +197,16 @@  void ctcmpc_dumpit(char *buf, int len)
 			strcat(basc, " ");
 		}
 		if (dup != 0) {
-			sprintf(tdup, "Duplicate as above to %s", addr);
+			scnprintf(tdup, sizeof(tdup),
+				  "Duplicate as above to %s", addr);
 			ctcm_pr_debug("		       --- %s ---\n", tdup);
 		}
 		ctcm_pr_debug("   %s (+%s) : %s  [%s]\n",
 					addr, boff, bhex, basc);
 	} else {
 		if (dup >= 1) {
-			sprintf(tdup, "Duplicate as above to %s", addr);
+			scnprintf(tdup, sizeof(tdup),
+				  "Duplicate as above to %s", addr);
 			ctcm_pr_debug("		       --- %s ---\n", tdup);
 		}
 		if (dup != 0) {
@@ -291,7 +293,7 @@  static struct net_device *ctcmpc_get_dev(int port_num)
 	struct net_device *dev;
 	struct ctcm_priv *priv;
 
-	sprintf(device, "%s%i", MPC_DEVICE_NAME, port_num);
+	scnprintf(device, sizeof(device), "%s%i", MPC_DEVICE_NAME, port_num);
 
 	dev = __dev_get_by_name(&init_net, device);
 
diff --git a/drivers/s390/net/ctcm_sysfs.c b/drivers/s390/net/ctcm_sysfs.c
index 98680c2cc4e4..0c5d8a3eaa2e 100644
--- a/drivers/s390/net/ctcm_sysfs.c
+++ b/drivers/s390/net/ctcm_sysfs.c
@@ -86,24 +86,24 @@  static void ctcm_print_statistics(struct ctcm_priv *priv)
 		return;
 	p = sbuf;
 
-	p += sprintf(p, "  Device FSM state: %s\n",
-		     fsm_getstate_str(priv->fsm));
-	p += sprintf(p, "  RX channel FSM state: %s\n",
-		     fsm_getstate_str(priv->channel[CTCM_READ]->fsm));
-	p += sprintf(p, "  TX channel FSM state: %s\n",
-		     fsm_getstate_str(priv->channel[CTCM_WRITE]->fsm));
-	p += sprintf(p, "  Max. TX buffer used: %ld\n",
-		     priv->channel[WRITE]->prof.maxmulti);
-	p += sprintf(p, "  Max. chained SKBs: %ld\n",
-		     priv->channel[WRITE]->prof.maxcqueue);
-	p += sprintf(p, "  TX single write ops: %ld\n",
-		     priv->channel[WRITE]->prof.doios_single);
-	p += sprintf(p, "  TX multi write ops: %ld\n",
-		     priv->channel[WRITE]->prof.doios_multi);
-	p += sprintf(p, "  Netto bytes written: %ld\n",
-		     priv->channel[WRITE]->prof.txlen);
-	p += sprintf(p, "  Max. TX IO-time: %u\n",
-		     jiffies_to_usecs(priv->channel[WRITE]->prof.tx_time));
+	p += scnprintf(p, CTCM_STATSIZE_LIMIT, "  Device FSM state: %s\n",
+		       fsm_getstate_str(priv->fsm));
+	p += scnprintf(p, CTCM_STATSIZE_LIMIT, "  RX channel FSM state: %s\n",
+		       fsm_getstate_str(priv->channel[CTCM_READ]->fsm));
+	p += scnprintf(p, CTCM_STATSIZE_LIMIT, "  TX channel FSM state: %s\n",
+		       fsm_getstate_str(priv->channel[CTCM_WRITE]->fsm));
+	p += scnprintf(p, CTCM_STATSIZE_LIMIT, "  Max. TX buffer used: %ld\n",
+		       priv->channel[WRITE]->prof.maxmulti);
+	p += scnprintf(p, CTCM_STATSIZE_LIMIT, "  Max. chained SKBs: %ld\n",
+		       priv->channel[WRITE]->prof.maxcqueue);
+	p += scnprintf(p, CTCM_STATSIZE_LIMIT, "  TX single write ops: %ld\n",
+		       priv->channel[WRITE]->prof.doios_single);
+	p += scnprintf(p, CTCM_STATSIZE_LIMIT, "  TX multi write ops: %ld\n",
+		       priv->channel[WRITE]->prof.doios_multi);
+	p += scnprintf(p, CTCM_STATSIZE_LIMIT, "  Netto bytes written: %ld\n",
+		       priv->channel[WRITE]->prof.txlen);
+	p += scnprintf(p, CTCM_STATSIZE_LIMIT, "  Max. TX IO-time: %u\n",
+		       jiffies_to_usecs(priv->channel[WRITE]->prof.tx_time));
 
 	printk(KERN_INFO "Statistics for %s:\n%s",
 				priv->channel[CTCM_WRITE]->netdev->name, sbuf);