diff mbox series

[15/42] drivers/ptp: Convert snprintf to sysfs_emit

Message ID 20240116045151.3940401-13-lizhijian@fujitsu.com
State New
Headers
Series [01/42] coccinelle: device_attr_show.cocci: update description and warning message |

Commit Message

Zhijian Li (Fujitsu) Jan. 16, 2024, 4:51 a.m. UTC 
  Per filesystems/sysfs.rst, show() should only use sysfs_emit()
or sysfs_emit_at() when formatting the value to be returned to user space.

coccinelle complains that there are still a couple of functions that use
snprintf(). Convert them to sysfs_emit().

> ./drivers/ptp/ptp_sysfs.c:27:8-16: WARNING: please use sysfs_emit

No functional change intended

CC: Richard Cochran <richardcochran@gmail.com>
CC: netdev@vger.kernel.org
Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
---
 drivers/ptp/ptp_sysfs.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Rahul Rameshbabu Jan. 16, 2024, 5:01 a.m. UTC | #1 
On Tue, 16 Jan, 2024 12:51:24 +0800 Li Zhijian <lizhijian@fujitsu.com> wrote:
> Per filesystems/sysfs.rst, show() should only use sysfs_emit()
> or sysfs_emit_at() when formatting the value to be returned to user space.
>
> coccinelle complains that there are still a couple of functions that use
> snprintf(). Convert them to sysfs_emit().
>
>> ./drivers/ptp/ptp_sysfs.c:27:8-16: WARNING: please use sysfs_emit
>
> No functional change intended
>
> CC: Richard Cochran <richardcochran@gmail.com>
> CC: netdev@vger.kernel.org
> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
> ---
>  drivers/ptp/ptp_sysfs.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/ptp/ptp_sysfs.c b/drivers/ptp/ptp_sysfs.c
> index f7a499a1bd39..49737ed6ef5f 100644
> --- a/drivers/ptp/ptp_sysfs.c
> +++ b/drivers/ptp/ptp_sysfs.c
> @@ -24,8 +24,7 @@ static ssize_t max_phase_adjustment_show(struct device *dev,
>  {
>  	struct ptp_clock *ptp = dev_get_drvdata(dev);
>  
> -	return snprintf(page, PAGE_SIZE - 1, "%d\n",
> -			ptp->info->getmaxphase(ptp->info));
> +	return sysfs_emit(page, "%d\n", ptp->info->getmaxphase(ptp->info));
>  }
>  static DEVICE_ATTR_RO(max_phase_adjustment);

I authored the lines that are being changed here, so figured I should
provide my review. Doesn't PTP_SHOW_INT in the same file also use
snprintf in the same manner and should be changed to sysfs_emit?

--
Thanks,

Rahul Rameshbabu
Zhijian Li (Fujitsu) Jan. 16, 2024, 5:52 a.m. UTC | #2 
On 16/01/2024 13:01, Rahul Rameshbabu wrote:
> On Tue, 16 Jan, 2024 12:51:24 +0800 Li Zhijian <lizhijian@fujitsu.com> wrote:
>> Per filesystems/sysfs.rst, show() should only use sysfs_emit()
>> or sysfs_emit_at() when formatting the value to be returned to user space.
>>
>> coccinelle complains that there are still a couple of functions that use
>> snprintf(). Convert them to sysfs_emit().
>>
>>> ./drivers/ptp/ptp_sysfs.c:27:8-16: WARNING: please use sysfs_emit
>>
>> No functional change intended
>>
>> CC: Richard Cochran <richardcochran@gmail.com>
>> CC: netdev@vger.kernel.org
>> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
>> ---
>>   drivers/ptp/ptp_sysfs.c | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/ptp/ptp_sysfs.c b/drivers/ptp/ptp_sysfs.c
>> index f7a499a1bd39..49737ed6ef5f 100644
>> --- a/drivers/ptp/ptp_sysfs.c
>> +++ b/drivers/ptp/ptp_sysfs.c
>> @@ -24,8 +24,7 @@ static ssize_t max_phase_adjustment_show(struct device *dev,
>>   {
>>   	struct ptp_clock *ptp = dev_get_drvdata(dev);
>>   
>> -	return snprintf(page, PAGE_SIZE - 1, "%d\n",
>> -			ptp->info->getmaxphase(ptp->info));
>> +	return sysfs_emit(page, "%d\n", ptp->info->getmaxphase(ptp->info));
>>   }
>>   static DEVICE_ATTR_RO(max_phase_adjustment);
> 
> I authored the lines that are being changed here, so figured I should
> provide my review. Doesn't PTP_SHOW_INT in the same file also use
> snprintf in the same manner and should be changed to sysfs_emit?

Thanks for you review.
Yes, i think so, beside PTP_SHOW_INT, there are 3 other places not detected by coccinelle should be fixed.
I will update it in V2.

$ grep snprintf drivers/ptp/ptp_sysfs.c
         return snprintf(page, PAGE_SIZE-1, "%d\n", ptp->info->var);     \
         cnt = snprintf(page, PAGE_SIZE, "%u %lld %u\n",
         size = snprintf(page, PAGE_SIZE - 1, "%u\n", ptp->n_vclocks);
         size = snprintf(page, PAGE_SIZE - 1, "%u\n", ptp->max_vclocks);


Thanks
Zhijian


> 
> --
> Thanks,
> 
> Rahul Rameshbabu
Jakub Kicinski Jan. 16, 2024, 3:33 p.m. UTC | #3 
On Tue, 16 Jan 2024 12:51:24 +0800 Li Zhijian wrote:
> Per filesystems/sysfs.rst, show() should only use sysfs_emit()
> or sysfs_emit_at() when formatting the value to be returned to user space.
> 
> coccinelle complains that there are still a couple of functions that use
> snprintf(). Convert them to sysfs_emit().
> 
> > ./drivers/ptp/ptp_sysfs.c:27:8-16: WARNING: please use sysfs_emit  

If the patches don't depend on each other please post them separately.
Series should only be used if there are dependencies and the same
maintainer is expected to apply all patches.

The ptp change should be reposted after the merge window, we don't take
cleanups during the merge window.
Zhijian Li (Fujitsu) Jan. 17, 2024, 5:39 a.m. UTC | #4 
Jakub,


On 16/01/2024 23:33, Jakub Kicinski wrote:
> On Tue, 16 Jan 2024 12:51:24 +0800 Li Zhijian wrote:
>> Per filesystems/sysfs.rst, show() should only use sysfs_emit()
>> or sysfs_emit_at() when formatting the value to be returned to user space.
>>
>> coccinelle complains that there are still a couple of functions that use
>> snprintf(). Convert them to sysfs_emit().
>>
>>> ./drivers/ptp/ptp_sysfs.c:27:8-16: WARNING: please use sysfs_emit
> 
> If the patches don't depend on each other please post them separately.
> Series should only be used if there are dependencies and the same
> maintainer is expected to apply all patches> 
> The ptp change should be reposted after the merge window, we don't take
> cleanups during the merge window.

Understood. i will split them per subsystem/maintainer and repost them
separately after the merge window.


Thanks
Zhijian

(Grouping them into the same set helped us have an overview of this warning.)
diff mbox series

Patch

diff --git a/drivers/ptp/ptp_sysfs.c b/drivers/ptp/ptp_sysfs.c
index f7a499a1bd39..49737ed6ef5f 100644
--- a/drivers/ptp/ptp_sysfs.c
+++ b/drivers/ptp/ptp_sysfs.c
@@ -24,8 +24,7 @@  static ssize_t max_phase_adjustment_show(struct device *dev,
 {
 	struct ptp_clock *ptp = dev_get_drvdata(dev);
 
-	return snprintf(page, PAGE_SIZE - 1, "%d\n",
-			ptp->info->getmaxphase(ptp->info));
+	return sysfs_emit(page, "%d\n", ptp->info->getmaxphase(ptp->info));
 }
 static DEVICE_ATTR_RO(max_phase_adjustment);