netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 net-next] tipc: adjust tipc_nodeid2string() to return string length
@ 2025-09-24 11:26 Dmitry Antipov
  2025-09-25  1:41 ` Tung Quang Nguyen
  2025-09-25 12:47 ` Simon Horman
  0 siblings, 2 replies; 10+ messages in thread
From: Dmitry Antipov @ 2025-09-24 11:26 UTC (permalink / raw)
  To: Jon Maloy
  Cc: David S . Miller, Jakub Kicinski, Tung Quang Nguyen,
	tipc-discussion, netdev, Dmitry Antipov

Since the value returned by 'tipc_nodeid2string()' is not used, the
function may be adjusted to return the length of the result, which
is helpful to drop a few calls to 'strlen()' in 'tipc_link_create()'
and 'tipc_link_bc_create()'. Compile tested only.

Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
v2: adjusted to target net-next (Tung Quang Nguyen)
---
 net/tipc/addr.c | 6 +++---
 net/tipc/addr.h | 2 +-
 net/tipc/link.c | 9 +++------
 3 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/net/tipc/addr.c b/net/tipc/addr.c
index fd0796269eed..6f5c54cbf8d9 100644
--- a/net/tipc/addr.c
+++ b/net/tipc/addr.c
@@ -79,7 +79,7 @@ void tipc_set_node_addr(struct net *net, u32 addr)
 	pr_info("Node number set to %u\n", addr);
 }
 
-char *tipc_nodeid2string(char *str, u8 *id)
+int tipc_nodeid2string(char *str, u8 *id)
 {
 	int i;
 	u8 c;
@@ -109,7 +109,7 @@ char *tipc_nodeid2string(char *str, u8 *id)
 	if (i == NODE_ID_LEN) {
 		memcpy(str, id, NODE_ID_LEN);
 		str[NODE_ID_LEN] = 0;
-		return str;
+		return i;
 	}
 
 	/* Translate to hex string */
@@ -120,5 +120,5 @@ char *tipc_nodeid2string(char *str, u8 *id)
 	for (i = NODE_ID_STR_LEN - 2; str[i] == '0'; i--)
 		str[i] = 0;
 
-	return str;
+	return i + 1;
 }
diff --git a/net/tipc/addr.h b/net/tipc/addr.h
index 93f82398283d..a113cf7e1f89 100644
--- a/net/tipc/addr.h
+++ b/net/tipc/addr.h
@@ -130,6 +130,6 @@ static inline int in_own_node(struct net *net, u32 addr)
 bool tipc_in_scope(bool legacy_format, u32 domain, u32 addr);
 void tipc_set_node_id(struct net *net, u8 *id);
 void tipc_set_node_addr(struct net *net, u32 addr);
-char *tipc_nodeid2string(char *str, u8 *id);
+int tipc_nodeid2string(char *str, u8 *id);
 
 #endif
diff --git a/net/tipc/link.c b/net/tipc/link.c
index 3ee44d731700..e61872b5b2b3 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -495,11 +495,9 @@ bool tipc_link_create(struct net *net, char *if_name, int bearer_id,
 
 	/* Set link name for unicast links only */
 	if (peer_id) {
-		tipc_nodeid2string(self_str, tipc_own_id(net));
-		if (strlen(self_str) > 16)
+		if (tipc_nodeid2string(self_str, tipc_own_id(net)) > 16)
 			sprintf(self_str, "%x", self);
-		tipc_nodeid2string(peer_str, peer_id);
-		if (strlen(peer_str) > 16)
+		if (tipc_nodeid2string(peer_str, peer_id) > 16)
 			sprintf(peer_str, "%x", peer);
 	}
 	/* Peer i/f name will be completed by reset/activate message */
@@ -570,8 +568,7 @@ bool tipc_link_bc_create(struct net *net, u32 ownnode, u32 peer, u8 *peer_id,
 	if (peer_id) {
 		char peer_str[NODE_ID_STR_LEN] = {0,};
 
-		tipc_nodeid2string(peer_str, peer_id);
-		if (strlen(peer_str) > 16)
+		if (tipc_nodeid2string(peer_str, peer_id) > 16)
 			sprintf(peer_str, "%x", peer);
 		/* Broadcast receiver link name: "broadcast-link:<peer>" */
 		snprintf(l->name, sizeof(l->name), "%s:%s", tipc_bclink_name,
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* RE: [PATCH v2 net-next] tipc: adjust tipc_nodeid2string() to return string length
  2025-09-24 11:26 [PATCH v2 net-next] tipc: adjust tipc_nodeid2string() to return string length Dmitry Antipov
@ 2025-09-25  1:41 ` Tung Quang Nguyen
  2025-09-25 12:47 ` Simon Horman
  1 sibling, 0 replies; 10+ messages in thread
From: Tung Quang Nguyen @ 2025-09-25  1:41 UTC (permalink / raw)
  To: Dmitry Antipov
  Cc: David S . Miller, Jakub Kicinski,
	tipc-discussion@lists.sourceforge.net, netdev@vger.kernel.org,
	Jon Maloy

>Subject: [PATCH v2 net-next] tipc: adjust tipc_nodeid2string() to return string
>length
>
>Since the value returned by 'tipc_nodeid2string()' is not used, the function may
>be adjusted to return the length of the result, which is helpful to drop a few
>calls to 'strlen()' in 'tipc_link_create()'
>and 'tipc_link_bc_create()'. Compile tested only.
>
>Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
>---
>v2: adjusted to target net-next (Tung Quang Nguyen)
>---
> net/tipc/addr.c | 6 +++---
> net/tipc/addr.h | 2 +-
> net/tipc/link.c | 9 +++------
> 3 files changed, 7 insertions(+), 10 deletions(-)
>
>diff --git a/net/tipc/addr.c b/net/tipc/addr.c index fd0796269eed..6f5c54cbf8d9
>100644
>--- a/net/tipc/addr.c
>+++ b/net/tipc/addr.c
>@@ -79,7 +79,7 @@ void tipc_set_node_addr(struct net *net, u32 addr)
> 	pr_info("Node number set to %u\n", addr);  }
>
>-char *tipc_nodeid2string(char *str, u8 *id)
>+int tipc_nodeid2string(char *str, u8 *id)
> {
> 	int i;
> 	u8 c;
>@@ -109,7 +109,7 @@ char *tipc_nodeid2string(char *str, u8 *id)
> 	if (i == NODE_ID_LEN) {
> 		memcpy(str, id, NODE_ID_LEN);
> 		str[NODE_ID_LEN] = 0;
>-		return str;
>+		return i;
> 	}
>
> 	/* Translate to hex string */
>@@ -120,5 +120,5 @@ char *tipc_nodeid2string(char *str, u8 *id)
> 	for (i = NODE_ID_STR_LEN - 2; str[i] == '0'; i--)
> 		str[i] = 0;
>
>-	return str;
>+	return i + 1;
> }
>diff --git a/net/tipc/addr.h b/net/tipc/addr.h index
>93f82398283d..a113cf7e1f89 100644
>--- a/net/tipc/addr.h
>+++ b/net/tipc/addr.h
>@@ -130,6 +130,6 @@ static inline int in_own_node(struct net *net, u32 addr)
>bool tipc_in_scope(bool legacy_format, u32 domain, u32 addr);  void
>tipc_set_node_id(struct net *net, u8 *id);  void tipc_set_node_addr(struct net
>*net, u32 addr); -char *tipc_nodeid2string(char *str, u8 *id);
>+int tipc_nodeid2string(char *str, u8 *id);
>
> #endif
>diff --git a/net/tipc/link.c b/net/tipc/link.c index 3ee44d731700..e61872b5b2b3
>100644
>--- a/net/tipc/link.c
>+++ b/net/tipc/link.c
>@@ -495,11 +495,9 @@ bool tipc_link_create(struct net *net, char *if_name,
>int bearer_id,
>
> 	/* Set link name for unicast links only */
> 	if (peer_id) {
>-		tipc_nodeid2string(self_str, tipc_own_id(net));
>-		if (strlen(self_str) > 16)
>+		if (tipc_nodeid2string(self_str, tipc_own_id(net)) > 16)
> 			sprintf(self_str, "%x", self);
>-		tipc_nodeid2string(peer_str, peer_id);
>-		if (strlen(peer_str) > 16)
>+		if (tipc_nodeid2string(peer_str, peer_id) > 16)
> 			sprintf(peer_str, "%x", peer);
> 	}
> 	/* Peer i/f name will be completed by reset/activate message */ @@ -
>570,8 +568,7 @@ bool tipc_link_bc_create(struct net *net, u32 ownnode, u32
>peer, u8 *peer_id,
> 	if (peer_id) {
> 		char peer_str[NODE_ID_STR_LEN] = {0,};
>
>-		tipc_nodeid2string(peer_str, peer_id);
>-		if (strlen(peer_str) > 16)
>+		if (tipc_nodeid2string(peer_str, peer_id) > 16)
> 			sprintf(peer_str, "%x", peer);
> 		/* Broadcast receiver link name: "broadcast-link:<peer>" */
> 		snprintf(l->name, sizeof(l->name), "%s:%s", tipc_bclink_name,
>--
>2.51.0
Reviewed-and-tested-by: Tung Nguyen <tung.quang.nguyen@est.tech>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 net-next] tipc: adjust tipc_nodeid2string() to return string length
  2025-09-24 11:26 [PATCH v2 net-next] tipc: adjust tipc_nodeid2string() to return string length Dmitry Antipov
  2025-09-25  1:41 ` Tung Quang Nguyen
@ 2025-09-25 12:47 ` Simon Horman
  2025-09-25 16:51   ` [PATCH v3 net-next] tipc: adjust tipc_nodeid2string() to check the length of the result Dmitry Antipov
  1 sibling, 1 reply; 10+ messages in thread
From: Simon Horman @ 2025-09-25 12:47 UTC (permalink / raw)
  To: Dmitry Antipov
  Cc: Jon Maloy, David S . Miller, Jakub Kicinski, Tung Quang Nguyen,
	tipc-discussion, netdev

On Wed, Sep 24, 2025 at 02:26:49PM +0300, Dmitry Antipov wrote:

...

> diff --git a/net/tipc/link.c b/net/tipc/link.c
> index 3ee44d731700..e61872b5b2b3 100644
> --- a/net/tipc/link.c
> +++ b/net/tipc/link.c
> @@ -495,11 +495,9 @@ bool tipc_link_create(struct net *net, char *if_name, int bearer_id,
>  
>  	/* Set link name for unicast links only */
>  	if (peer_id) {
> -		tipc_nodeid2string(self_str, tipc_own_id(net));
> -		if (strlen(self_str) > 16)
> +		if (tipc_nodeid2string(self_str, tipc_own_id(net)) > 16)
>  			sprintf(self_str, "%x", self);

I see this is a nice clean-up. Thanks.

Would it make sense to move the '> 16' logic also be folded into
tipc_nodeid2string(), or a wrapper provided, to further simplify the
callers?

> -		tipc_nodeid2string(peer_str, peer_id);
> -		if (strlen(peer_str) > 16)
> +		if (tipc_nodeid2string(peer_str, peer_id) > 16)
>  			sprintf(peer_str, "%x", peer);
>  	}
>  	/* Peer i/f name will be completed by reset/activate message */
> @@ -570,8 +568,7 @@ bool tipc_link_bc_create(struct net *net, u32 ownnode, u32 peer, u8 *peer_id,
>  	if (peer_id) {
>  		char peer_str[NODE_ID_STR_LEN] = {0,};
>  
> -		tipc_nodeid2string(peer_str, peer_id);
> -		if (strlen(peer_str) > 16)
> +		if (tipc_nodeid2string(peer_str, peer_id) > 16)
>  			sprintf(peer_str, "%x", peer);
>  		/* Broadcast receiver link name: "broadcast-link:<peer>" */
>  		snprintf(l->name, sizeof(l->name), "%s:%s", tipc_bclink_name,
> -- 
> 2.51.0
> 
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 net-next] tipc: adjust tipc_nodeid2string() to check the length of the result
  2025-09-25 12:47 ` Simon Horman
@ 2025-09-25 16:51   ` Dmitry Antipov
  2025-09-26  3:34     ` Tung Quang Nguyen
  2025-09-26 15:39     ` [PATCH v3 net-next] tipc: adjust tipc_nodeid2string() to check the length of the result Simon Horman
  0 siblings, 2 replies; 10+ messages in thread
From: Dmitry Antipov @ 2025-09-25 16:51 UTC (permalink / raw)
  To: Simon Horman
  Cc: Jon Maloy, David S . Miller, Jakub Kicinski, Tung Quang Nguyen,
	tipc-discussion, netdev, Dmitry Antipov

Since the value returned by 'tipc_nodeid2string()' is not used, the
function may be adjusted to check the length of the result against
NODE_ID_LEN, which is helpful to drop a few calls to 'strlen()' and
simplify 'tipc_link_create()' and 'tipc_link_bc_create()'. Compile
tested only.

Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
v3: convert to check against NODE_ID_LEN (Simon Horman)
v2: adjusted to target net-next (Tung Quang Nguyen)
---
 net/tipc/addr.c | 6 +++---
 net/tipc/addr.h | 2 +-
 net/tipc/link.c | 9 +++------
 3 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/net/tipc/addr.c b/net/tipc/addr.c
index fd0796269eed..90e47add376e 100644
--- a/net/tipc/addr.c
+++ b/net/tipc/addr.c
@@ -79,7 +79,7 @@ void tipc_set_node_addr(struct net *net, u32 addr)
 	pr_info("Node number set to %u\n", addr);
 }
 
-char *tipc_nodeid2string(char *str, u8 *id)
+bool tipc_nodeid2string(char *str, u8 *id)
 {
 	int i;
 	u8 c;
@@ -109,7 +109,7 @@ char *tipc_nodeid2string(char *str, u8 *id)
 	if (i == NODE_ID_LEN) {
 		memcpy(str, id, NODE_ID_LEN);
 		str[NODE_ID_LEN] = 0;
-		return str;
+		return false;
 	}
 
 	/* Translate to hex string */
@@ -120,5 +120,5 @@ char *tipc_nodeid2string(char *str, u8 *id)
 	for (i = NODE_ID_STR_LEN - 2; str[i] == '0'; i--)
 		str[i] = 0;
 
-	return str;
+	return i + 1 > NODE_ID_LEN;
 }
diff --git a/net/tipc/addr.h b/net/tipc/addr.h
index 93f82398283d..5e4fc27fe329 100644
--- a/net/tipc/addr.h
+++ b/net/tipc/addr.h
@@ -130,6 +130,6 @@ static inline int in_own_node(struct net *net, u32 addr)
 bool tipc_in_scope(bool legacy_format, u32 domain, u32 addr);
 void tipc_set_node_id(struct net *net, u8 *id);
 void tipc_set_node_addr(struct net *net, u32 addr);
-char *tipc_nodeid2string(char *str, u8 *id);
+bool tipc_nodeid2string(char *str, u8 *id);
 
 #endif
diff --git a/net/tipc/link.c b/net/tipc/link.c
index 3ee44d731700..93181b1d8898 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -495,11 +495,9 @@ bool tipc_link_create(struct net *net, char *if_name, int bearer_id,
 
 	/* Set link name for unicast links only */
 	if (peer_id) {
-		tipc_nodeid2string(self_str, tipc_own_id(net));
-		if (strlen(self_str) > 16)
+		if (tipc_nodeid2string(self_str, tipc_own_id(net)))
 			sprintf(self_str, "%x", self);
-		tipc_nodeid2string(peer_str, peer_id);
-		if (strlen(peer_str) > 16)
+		if (tipc_nodeid2string(peer_str, peer_id))
 			sprintf(peer_str, "%x", peer);
 	}
 	/* Peer i/f name will be completed by reset/activate message */
@@ -570,8 +568,7 @@ bool tipc_link_bc_create(struct net *net, u32 ownnode, u32 peer, u8 *peer_id,
 	if (peer_id) {
 		char peer_str[NODE_ID_STR_LEN] = {0,};
 
-		tipc_nodeid2string(peer_str, peer_id);
-		if (strlen(peer_str) > 16)
+		if (tipc_nodeid2string(peer_str, peer_id))
 			sprintf(peer_str, "%x", peer);
 		/* Broadcast receiver link name: "broadcast-link:<peer>" */
 		snprintf(l->name, sizeof(l->name), "%s:%s", tipc_bclink_name,
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* RE: [PATCH v3 net-next] tipc: adjust tipc_nodeid2string() to check the length of the result
  2025-09-25 16:51   ` [PATCH v3 net-next] tipc: adjust tipc_nodeid2string() to check the length of the result Dmitry Antipov
@ 2025-09-26  3:34     ` Tung Quang Nguyen
  2025-09-26  7:41       ` [PATCH v4 net-next] tipc: adjust tipc_nodeid2string() to return string length Dmitry Antipov
  2025-09-26 15:39     ` [PATCH v3 net-next] tipc: adjust tipc_nodeid2string() to check the length of the result Simon Horman
  1 sibling, 1 reply; 10+ messages in thread
From: Tung Quang Nguyen @ 2025-09-26  3:34 UTC (permalink / raw)
  To: Dmitry Antipov
  Cc: Jon Maloy, David S . Miller, Jakub Kicinski,
	tipc-discussion@lists.sourceforge.net, netdev@vger.kernel.org,
	Simon Horman

>Subject: [PATCH v3 net-next] tipc: adjust tipc_nodeid2string() to check the
>length of the result
>
>Since the value returned by 'tipc_nodeid2string()' is not used, the function may
>be adjusted to check the length of the result against NODE_ID_LEN, which is
>helpful to drop a few calls to 'strlen()' and simplify 'tipc_link_create()' and
>'tipc_link_bc_create()'. Compile tested only.
>
>Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
>---
>v3: convert to check against NODE_ID_LEN (Simon Horman)
>v2: adjusted to target net-next (Tung Quang Nguyen)
>---
> net/tipc/addr.c | 6 +++---
> net/tipc/addr.h | 2 +-
> net/tipc/link.c | 9 +++------
> 3 files changed, 7 insertions(+), 10 deletions(-)
>
>diff --git a/net/tipc/addr.c b/net/tipc/addr.c index
>fd0796269eed..90e47add376e 100644
>--- a/net/tipc/addr.c
>+++ b/net/tipc/addr.c
>@@ -79,7 +79,7 @@ void tipc_set_node_addr(struct net *net, u32 addr)
> 	pr_info("Node number set to %u\n", addr);  }
>
>-char *tipc_nodeid2string(char *str, u8 *id)
>+bool tipc_nodeid2string(char *str, u8 *id)
> {
> 	int i;
> 	u8 c;
>@@ -109,7 +109,7 @@ char *tipc_nodeid2string(char *str, u8 *id)
> 	if (i == NODE_ID_LEN) {
> 		memcpy(str, id, NODE_ID_LEN);
> 		str[NODE_ID_LEN] = 0;
>-		return str;
>+		return false;
> 	}
>
> 	/* Translate to hex string */
>@@ -120,5 +120,5 @@ char *tipc_nodeid2string(char *str, u8 *id)
> 	for (i = NODE_ID_STR_LEN - 2; str[i] == '0'; i--)
> 		str[i] = 0;
>
>-	return str;
>+	return i + 1 > NODE_ID_LEN;
No, you should not do this.
Firstly, this makes the function look vague. tipc_nodeid2string() converts node id to string and its return value if needed should be converted string length as you did in V2 patch.
Secondly, this adds unnecessary overhead in case we use node id that is less than 16 characters in length (For example, tipc_node_create() calls tipc_nodeid2string() without caring its return value).
So, just let callers of tipc_nodeid2string() decide what value they want to compare to.
I think you can improve (in V4) by replacing 16 with NODE_ID_LEN to make it more descriptive (in tipc_link_create() and tipc_link_bc_create()).

> }
>diff --git a/net/tipc/addr.h b/net/tipc/addr.h index 93f82398283d..5e4fc27fe329
>100644
>--- a/net/tipc/addr.h
>+++ b/net/tipc/addr.h
>@@ -130,6 +130,6 @@ static inline int in_own_node(struct net *net, u32 addr)
>bool tipc_in_scope(bool legacy_format, u32 domain, u32 addr);  void
>tipc_set_node_id(struct net *net, u8 *id);  void tipc_set_node_addr(struct net
>*net, u32 addr); -char *tipc_nodeid2string(char *str, u8 *id);
>+bool tipc_nodeid2string(char *str, u8 *id);
>
> #endif
>diff --git a/net/tipc/link.c b/net/tipc/link.c index 3ee44d731700..93181b1d8898
>100644
>--- a/net/tipc/link.c
>+++ b/net/tipc/link.c
>@@ -495,11 +495,9 @@ bool tipc_link_create(struct net *net, char *if_name,
>int bearer_id,
>
> 	/* Set link name for unicast links only */
> 	if (peer_id) {
>-		tipc_nodeid2string(self_str, tipc_own_id(net));
>-		if (strlen(self_str) > 16)
>+		if (tipc_nodeid2string(self_str, tipc_own_id(net)))
> 			sprintf(self_str, "%x", self);
>-		tipc_nodeid2string(peer_str, peer_id);
>-		if (strlen(peer_str) > 16)
>+		if (tipc_nodeid2string(peer_str, peer_id))
> 			sprintf(peer_str, "%x", peer);
> 	}
> 	/* Peer i/f name will be completed by reset/activate message */ @@ -
>570,8 +568,7 @@ bool tipc_link_bc_create(struct net *net, u32 ownnode, u32
>peer, u8 *peer_id,
> 	if (peer_id) {
> 		char peer_str[NODE_ID_STR_LEN] = {0,};
>
>-		tipc_nodeid2string(peer_str, peer_id);
>-		if (strlen(peer_str) > 16)
>+		if (tipc_nodeid2string(peer_str, peer_id))
> 			sprintf(peer_str, "%x", peer);
> 		/* Broadcast receiver link name: "broadcast-link:<peer>" */
> 		snprintf(l->name, sizeof(l->name), "%s:%s", tipc_bclink_name,
>--
>2.51.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v4 net-next] tipc: adjust tipc_nodeid2string() to return string length
  2025-09-26  3:34     ` Tung Quang Nguyen
@ 2025-09-26  7:41       ` Dmitry Antipov
  2025-09-26 10:29         ` Tung Quang Nguyen
  2025-09-30  9:40         ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 10+ messages in thread
From: Dmitry Antipov @ 2025-09-26  7:41 UTC (permalink / raw)
  To: Tung Quang Nguyen
  Cc: Simon Horman, Jon Maloy, David S . Miller, Jakub Kicinski,
	tipc-discussion, netdev, Dmitry Antipov

Since the value returned by 'tipc_nodeid2string()' is not used, the
function may be adjusted to return the length of the result, which
is helpful to drop a few calls to 'strlen()' in 'tipc_link_create()'
and 'tipc_link_bc_create()'. Compile tested only.

Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
v4: revert to v2's behavior and prefer NODE_ID_LEN over
    explicit constant (Tung Quang Nguyen)
v3: convert to check against NODE_ID_LEN (Simon Horman)
v2: adjusted to target net-next (Tung Quang Nguyen)
---
 net/tipc/addr.c | 6 +++---
 net/tipc/addr.h | 2 +-
 net/tipc/link.c | 9 +++------
 3 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/net/tipc/addr.c b/net/tipc/addr.c
index fd0796269eed..6f5c54cbf8d9 100644
--- a/net/tipc/addr.c
+++ b/net/tipc/addr.c
@@ -79,7 +79,7 @@ void tipc_set_node_addr(struct net *net, u32 addr)
 	pr_info("Node number set to %u\n", addr);
 }
 
-char *tipc_nodeid2string(char *str, u8 *id)
+int tipc_nodeid2string(char *str, u8 *id)
 {
 	int i;
 	u8 c;
@@ -109,7 +109,7 @@ char *tipc_nodeid2string(char *str, u8 *id)
 	if (i == NODE_ID_LEN) {
 		memcpy(str, id, NODE_ID_LEN);
 		str[NODE_ID_LEN] = 0;
-		return str;
+		return i;
 	}
 
 	/* Translate to hex string */
@@ -120,5 +120,5 @@ char *tipc_nodeid2string(char *str, u8 *id)
 	for (i = NODE_ID_STR_LEN - 2; str[i] == '0'; i--)
 		str[i] = 0;
 
-	return str;
+	return i + 1;
 }
diff --git a/net/tipc/addr.h b/net/tipc/addr.h
index 93f82398283d..a113cf7e1f89 100644
--- a/net/tipc/addr.h
+++ b/net/tipc/addr.h
@@ -130,6 +130,6 @@ static inline int in_own_node(struct net *net, u32 addr)
 bool tipc_in_scope(bool legacy_format, u32 domain, u32 addr);
 void tipc_set_node_id(struct net *net, u8 *id);
 void tipc_set_node_addr(struct net *net, u32 addr);
-char *tipc_nodeid2string(char *str, u8 *id);
+int tipc_nodeid2string(char *str, u8 *id);
 
 #endif
diff --git a/net/tipc/link.c b/net/tipc/link.c
index 3ee44d731700..931f55f781a1 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -495,11 +495,9 @@ bool tipc_link_create(struct net *net, char *if_name, int bearer_id,
 
 	/* Set link name for unicast links only */
 	if (peer_id) {
-		tipc_nodeid2string(self_str, tipc_own_id(net));
-		if (strlen(self_str) > 16)
+		if (tipc_nodeid2string(self_str, tipc_own_id(net)) > NODE_ID_LEN)
 			sprintf(self_str, "%x", self);
-		tipc_nodeid2string(peer_str, peer_id);
-		if (strlen(peer_str) > 16)
+		if (tipc_nodeid2string(peer_str, peer_id) > NODE_ID_LEN)
 			sprintf(peer_str, "%x", peer);
 	}
 	/* Peer i/f name will be completed by reset/activate message */
@@ -570,8 +568,7 @@ bool tipc_link_bc_create(struct net *net, u32 ownnode, u32 peer, u8 *peer_id,
 	if (peer_id) {
 		char peer_str[NODE_ID_STR_LEN] = {0,};
 
-		tipc_nodeid2string(peer_str, peer_id);
-		if (strlen(peer_str) > 16)
+		if (tipc_nodeid2string(peer_str, peer_id) > NODE_ID_LEN)
 			sprintf(peer_str, "%x", peer);
 		/* Broadcast receiver link name: "broadcast-link:<peer>" */
 		snprintf(l->name, sizeof(l->name), "%s:%s", tipc_bclink_name,
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* RE: [PATCH v4 net-next] tipc: adjust tipc_nodeid2string() to return string length
  2025-09-26  7:41       ` [PATCH v4 net-next] tipc: adjust tipc_nodeid2string() to return string length Dmitry Antipov
@ 2025-09-26 10:29         ` Tung Quang Nguyen
  2025-09-26 15:41           ` Simon Horman
  2025-09-30  9:40         ` patchwork-bot+netdevbpf
  1 sibling, 1 reply; 10+ messages in thread
From: Tung Quang Nguyen @ 2025-09-26 10:29 UTC (permalink / raw)
  To: Dmitry Antipov
  Cc: Simon Horman, Jon Maloy, David S . Miller, Jakub Kicinski,
	tipc-discussion@lists.sourceforge.net, netdev@vger.kernel.org

>Subject: [PATCH v4 net-next] tipc: adjust tipc_nodeid2string() to return string
>length
>
>Since the value returned by 'tipc_nodeid2string()' is not used, the function may
>be adjusted to return the length of the result, which is helpful to drop a few
>calls to 'strlen()' in 'tipc_link_create()'
>and 'tipc_link_bc_create()'. Compile tested only.
>
>Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
>---
>v4: revert to v2's behavior and prefer NODE_ID_LEN over
>    explicit constant (Tung Quang Nguyen)
>v3: convert to check against NODE_ID_LEN (Simon Horman)
>v2: adjusted to target net-next (Tung Quang Nguyen)
>---
> net/tipc/addr.c | 6 +++---
> net/tipc/addr.h | 2 +-
> net/tipc/link.c | 9 +++------
> 3 files changed, 7 insertions(+), 10 deletions(-)
>
>diff --git a/net/tipc/addr.c b/net/tipc/addr.c index fd0796269eed..6f5c54cbf8d9
>100644
>--- a/net/tipc/addr.c
>+++ b/net/tipc/addr.c
>@@ -79,7 +79,7 @@ void tipc_set_node_addr(struct net *net, u32 addr)
> 	pr_info("Node number set to %u\n", addr);  }
>
>-char *tipc_nodeid2string(char *str, u8 *id)
>+int tipc_nodeid2string(char *str, u8 *id)
> {
> 	int i;
> 	u8 c;
>@@ -109,7 +109,7 @@ char *tipc_nodeid2string(char *str, u8 *id)
> 	if (i == NODE_ID_LEN) {
> 		memcpy(str, id, NODE_ID_LEN);
> 		str[NODE_ID_LEN] = 0;
>-		return str;
>+		return i;
> 	}
>
> 	/* Translate to hex string */
>@@ -120,5 +120,5 @@ char *tipc_nodeid2string(char *str, u8 *id)
> 	for (i = NODE_ID_STR_LEN - 2; str[i] == '0'; i--)
> 		str[i] = 0;
>
>-	return str;
>+	return i + 1;
> }
>diff --git a/net/tipc/addr.h b/net/tipc/addr.h index
>93f82398283d..a113cf7e1f89 100644
>--- a/net/tipc/addr.h
>+++ b/net/tipc/addr.h
>@@ -130,6 +130,6 @@ static inline int in_own_node(struct net *net, u32 addr)
>bool tipc_in_scope(bool legacy_format, u32 domain, u32 addr);  void
>tipc_set_node_id(struct net *net, u8 *id);  void tipc_set_node_addr(struct net
>*net, u32 addr); -char *tipc_nodeid2string(char *str, u8 *id);
>+int tipc_nodeid2string(char *str, u8 *id);
>
> #endif
>diff --git a/net/tipc/link.c b/net/tipc/link.c index 3ee44d731700..931f55f781a1
>100644
>--- a/net/tipc/link.c
>+++ b/net/tipc/link.c
>@@ -495,11 +495,9 @@ bool tipc_link_create(struct net *net, char *if_name,
>int bearer_id,
>
> 	/* Set link name for unicast links only */
> 	if (peer_id) {
>-		tipc_nodeid2string(self_str, tipc_own_id(net));
>-		if (strlen(self_str) > 16)
>+		if (tipc_nodeid2string(self_str, tipc_own_id(net)) >
>NODE_ID_LEN)
> 			sprintf(self_str, "%x", self);
>-		tipc_nodeid2string(peer_str, peer_id);
>-		if (strlen(peer_str) > 16)
>+		if (tipc_nodeid2string(peer_str, peer_id) > NODE_ID_LEN)
> 			sprintf(peer_str, "%x", peer);
> 	}
> 	/* Peer i/f name will be completed by reset/activate message */ @@ -
>570,8 +568,7 @@ bool tipc_link_bc_create(struct net *net, u32 ownnode, u32
>peer, u8 *peer_id,
> 	if (peer_id) {
> 		char peer_str[NODE_ID_STR_LEN] = {0,};
>
>-		tipc_nodeid2string(peer_str, peer_id);
>-		if (strlen(peer_str) > 16)
>+		if (tipc_nodeid2string(peer_str, peer_id) > NODE_ID_LEN)
> 			sprintf(peer_str, "%x", peer);
> 		/* Broadcast receiver link name: "broadcast-link:<peer>" */
> 		snprintf(l->name, sizeof(l->name), "%s:%s", tipc_bclink_name,
>--
>2.51.0
Reviewed-and-tested-by: Tung Nguyen <tung.quang.nguyen@est.tech>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 net-next] tipc: adjust tipc_nodeid2string() to check the length of the result
  2025-09-25 16:51   ` [PATCH v3 net-next] tipc: adjust tipc_nodeid2string() to check the length of the result Dmitry Antipov
  2025-09-26  3:34     ` Tung Quang Nguyen
@ 2025-09-26 15:39     ` Simon Horman
  1 sibling, 0 replies; 10+ messages in thread
From: Simon Horman @ 2025-09-26 15:39 UTC (permalink / raw)
  To: Dmitry Antipov
  Cc: Jon Maloy, David S . Miller, Jakub Kicinski, Tung Quang Nguyen,
	tipc-discussion, netdev

On Thu, Sep 25, 2025 at 07:51:46PM +0300, Dmitry Antipov wrote:
> Since the value returned by 'tipc_nodeid2string()' is not used, the
> function may be adjusted to check the length of the result against
> NODE_ID_LEN, which is helpful to drop a few calls to 'strlen()' and
> simplify 'tipc_link_create()' and 'tipc_link_bc_create()'. Compile
> tested only.
> 
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
> ---
> v3: convert to check against NODE_ID_LEN (Simon Horman)
> v2: adjusted to target net-next (Tung Quang Nguyen)

Reviewed-by: Simon Horman <horms@kernel.org>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 net-next] tipc: adjust tipc_nodeid2string() to return string length
  2025-09-26 10:29         ` Tung Quang Nguyen
@ 2025-09-26 15:41           ` Simon Horman
  0 siblings, 0 replies; 10+ messages in thread
From: Simon Horman @ 2025-09-26 15:41 UTC (permalink / raw)
  To: Tung Quang Nguyen
  Cc: Dmitry Antipov, Jon Maloy, David S . Miller, Jakub Kicinski,
	tipc-discussion@lists.sourceforge.net, netdev@vger.kernel.org

On Fri, Sep 26, 2025 at 10:29:31AM +0000, Tung Quang Nguyen wrote:
> >Subject: [PATCH v4 net-next] tipc: adjust tipc_nodeid2string() to return string
> >length
> >
> >Since the value returned by 'tipc_nodeid2string()' is not used, the function may
> >be adjusted to return the length of the result, which is helpful to drop a few
> >calls to 'strlen()' in 'tipc_link_create()'
> >and 'tipc_link_bc_create()'. Compile tested only.
> >
> >Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
> >---
> >v4: revert to v2's behavior and prefer NODE_ID_LEN over
> >    explicit constant (Tung Quang Nguyen)
> >v3: convert to check against NODE_ID_LEN (Simon Horman)
> >v2: adjusted to target net-next (Tung Quang Nguyen)

Reviewed-by: Simon Horman <horms@kernel.org>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 net-next] tipc: adjust tipc_nodeid2string() to return string length
  2025-09-26  7:41       ` [PATCH v4 net-next] tipc: adjust tipc_nodeid2string() to return string length Dmitry Antipov
  2025-09-26 10:29         ` Tung Quang Nguyen
@ 2025-09-30  9:40         ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 10+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-09-30  9:40 UTC (permalink / raw)
  To: Dmitry Antipov
  Cc: tung.quang.nguyen, horms, jmaloy, davem, kuba, tipc-discussion,
	netdev

Hello:

This patch was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Fri, 26 Sep 2025 10:41:13 +0300 you wrote:
> Since the value returned by 'tipc_nodeid2string()' is not used, the
> function may be adjusted to return the length of the result, which
> is helpful to drop a few calls to 'strlen()' in 'tipc_link_create()'
> and 'tipc_link_bc_create()'. Compile tested only.
> 
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
> 
> [...]

Here is the summary with links:
  - [v4,net-next] tipc: adjust tipc_nodeid2string() to return string length
    https://git.kernel.org/netdev/net-next/c/2ade91705b59

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2025-09-30  9:40 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-24 11:26 [PATCH v2 net-next] tipc: adjust tipc_nodeid2string() to return string length Dmitry Antipov
2025-09-25  1:41 ` Tung Quang Nguyen
2025-09-25 12:47 ` Simon Horman
2025-09-25 16:51   ` [PATCH v3 net-next] tipc: adjust tipc_nodeid2string() to check the length of the result Dmitry Antipov
2025-09-26  3:34     ` Tung Quang Nguyen
2025-09-26  7:41       ` [PATCH v4 net-next] tipc: adjust tipc_nodeid2string() to return string length Dmitry Antipov
2025-09-26 10:29         ` Tung Quang Nguyen
2025-09-26 15:41           ` Simon Horman
2025-09-30  9:40         ` patchwork-bot+netdevbpf
2025-09-26 15:39     ` [PATCH v3 net-next] tipc: adjust tipc_nodeid2string() to check the length of the result Simon Horman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).