[PATCH linux-next] net: atm: use sysfs_emit()/sysfs_emit_at() instead of scnprintf().

Posted by xie.ludan@zte.com.cn 9 months ago

From: XieLudan <xie.ludan@zte.com.cn>

Follow the advice in Documentation/filesystems/sysfs.rst:
show() should only use sysfs_emit() or sysfs_emit_at() when formatting
the value to be returned to user space.

Signed-off-by: XieLudan <xie.ludan@zte.com.cn>
---
 net/atm/atm_sysfs.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/atm/atm_sysfs.c b/net/atm/atm_sysfs.c
index 54e7fb1a4ee5..ae0d921157c5 100644
--- a/net/atm/atm_sysfs.c
+++ b/net/atm/atm_sysfs.c
@@ -16,7 +16,7 @@ static ssize_t type_show(struct device *cdev,
 {
 	struct atm_dev *adev = to_atm_dev(cdev);

-	return scnprintf(buf, PAGE_SIZE, "%s\n", adev->type);
+	return sysfs_emit(buf, "%s\n", adev->type);
 }

 static ssize_t address_show(struct device *cdev,
@@ -24,7 +24,7 @@ static ssize_t address_show(struct device *cdev,
 {
 	struct atm_dev *adev = to_atm_dev(cdev);

-	return scnprintf(buf, PAGE_SIZE, "%pM\n", adev->esi);
+	return sysfs_emit(buf, "%pM\n", adev->esi);
 }

 static ssize_t atmaddress_show(struct device *cdev,
@@ -37,7 +37,7 @@ static ssize_t atmaddress_show(struct device *cdev,

 	spin_lock_irqsave(&adev->lock, flags);
 	list_for_each_entry(aaddr, &adev->local, entry) {
-		count += scnprintf(buf + count, PAGE_SIZE - count,
+		count += sysfs_emit_at(buf, count,
 				   "%1phN.%2phN.%10phN.%6phN.%1phN\n",
 				   &aaddr->addr.sas_addr.prv[0],
 				   &aaddr->addr.sas_addr.prv[1],
@@ -55,7 +55,7 @@ static ssize_t atmindex_show(struct device *cdev,
 {
 	struct atm_dev *adev = to_atm_dev(cdev);

-	return scnprintf(buf, PAGE_SIZE, "%d\n", adev->number);
+	return sysfs_emit(buf, "%d\n", adev->number);
 }

 static ssize_t carrier_show(struct device *cdev,
@@ -63,7 +63,7 @@ static ssize_t carrier_show(struct device *cdev,
 {
 	struct atm_dev *adev = to_atm_dev(cdev);

-	return scnprintf(buf, PAGE_SIZE, "%d\n",
+	return sysfs_emit(buf, "%d\n",
 			 adev->signal == ATM_PHY_SIG_LOST ? 0 : 1);
 }

@@ -87,7 +87,7 @@ static ssize_t link_rate_show(struct device *cdev,
 	default:
 		link_rate = adev->link_rate * 8 * 53;
 	}
-	return scnprintf(buf, PAGE_SIZE, "%d\n", link_rate);
+	return sysfs_emit(buf, "%d\n", link_rate);
 }

 static DEVICE_ATTR_RO(address);
-- 
2.25.1

Re: [PATCH linux-next] net: atm: use sysfs_emit()/sysfs_emit_at() instead of scnprintf().

Posted by Simon Horman 9 months ago

On Mon, Mar 17, 2025 at 03:29:33PM +0800, xie.ludan@zte.com.cn wrote:
> From: XieLudan <xie.ludan@zte.com.cn>
> 
> Follow the advice in Documentation/filesystems/sysfs.rst:
> show() should only use sysfs_emit() or sysfs_emit_at() when formatting
> the value to be returned to user space.
> 
> Signed-off-by: XieLudan <xie.ludan@zte.com.cn>

Thanks Xie Ludan,

As per or discussion offline,
please coordinate with your colleague
Tang Dongxing who has also posted a patch in this area.

  https://lore.kernel.org/r/20250317155102808MZdMkiovw52X0oY7n47wI@zte.com.cn/

It will be much easier for review if there is a single patch
that addresses these issues for ATM.

Also, please consider reading the following guidance on processes
for the networking subsystem of the Linux kernel. These are similar
but different to other subsystems.

  https://docs.kernel.org/process/maintainer-netdev.html

--
pw-bot: changes-requested

Re: [PATCH linux-next] net: atm: use sysfs_emit()/sysfs_emit_at() instead of scnprintf().

Posted by Gerhard Engleder 9 months ago

On 17.03.25 08:29, xie.ludan@zte.com.cn wrote:
> From: XieLudan <xie.ludan@zte.com.cn>
> 
> Follow the advice in Documentation/filesystems/sysfs.rst:
> show() should only use sysfs_emit() or sysfs_emit_at() when formatting
> the value to be returned to user space.
> 
> Signed-off-by: XieLudan <xie.ludan@zte.com.cn>
> ---
>   net/atm/atm_sysfs.c | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/net/atm/atm_sysfs.c b/net/atm/atm_sysfs.c
> index 54e7fb1a4ee5..ae0d921157c5 100644
> --- a/net/atm/atm_sysfs.c
> +++ b/net/atm/atm_sysfs.c
> @@ -16,7 +16,7 @@ static ssize_t type_show(struct device *cdev,
>   {
>   	struct atm_dev *adev = to_atm_dev(cdev);
> 
> -	return scnprintf(buf, PAGE_SIZE, "%s\n", adev->type);
> +	return sysfs_emit(buf, "%s\n", adev->type);
>   }
> 
>   static ssize_t address_show(struct device *cdev,
> @@ -24,7 +24,7 @@ static ssize_t address_show(struct device *cdev,
>   {
>   	struct atm_dev *adev = to_atm_dev(cdev);
> 
> -	return scnprintf(buf, PAGE_SIZE, "%pM\n", adev->esi);
> +	return sysfs_emit(buf, "%pM\n", adev->esi);
>   }
> 
>   static ssize_t atmaddress_show(struct device *cdev,
> @@ -37,7 +37,7 @@ static ssize_t atmaddress_show(struct device *cdev,
> 
>   	spin_lock_irqsave(&adev->lock, flags);
>   	list_for_each_entry(aaddr, &adev->local, entry) {
> -		count += scnprintf(buf + count, PAGE_SIZE - count,
> +		count += sysfs_emit_at(buf, count,
>   				   "%1phN.%2phN.%10phN.%6phN.%1phN\n",
>   				   &aaddr->addr.sas_addr.prv[0],
>   				   &aaddr->addr.sas_addr.prv[1],

Does the alignment of the following argument lines needs to be adapted?

> @@ -55,7 +55,7 @@ static ssize_t atmindex_show(struct device *cdev,
>   {
>   	struct atm_dev *adev = to_atm_dev(cdev);
> 
> -	return scnprintf(buf, PAGE_SIZE, "%d\n", adev->number);
> +	return sysfs_emit(buf, "%d\n", adev->number);
>   }
> 
>   static ssize_t carrier_show(struct device *cdev,
> @@ -63,7 +63,7 @@ static ssize_t carrier_show(struct device *cdev,
>   {
>   	struct atm_dev *adev = to_atm_dev(cdev);
> 
> -	return scnprintf(buf, PAGE_SIZE, "%d\n",
> +	return sysfs_emit(buf, "%d\n",
>   			 adev->signal == ATM_PHY_SIG_LOST ? 0 : 1);

Adapt alignment of following line?

Gerhard