[PATCH] bus: mhi: core: replace snprintf with sysfs_emit

[PATCH] bus: mhi: core: replace snprintf with sysfs_emit

[Yihao Han]

coccinelle report:
./drivers/bus/mhi/core/init.c:97:8-16:
WARNING: use scnprintf or sprintf
Use sysfs_emit instead of scnprintf or sprintf makes more sense.

Signed-off-by: Yihao Han <hanyihao@vivo.com>
---
 drivers/bus/mhi/core/init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
index 5aaca6d0f52b..a5a5c722731e 100644
--- a/drivers/bus/mhi/core/init.c
+++ b/drivers/bus/mhi/core/init.c
@@ -94,7 +94,7 @@ static ssize_t serial_number_show(struct device *dev,
 	struct mhi_device *mhi_dev = to_mhi_device(dev);
 	struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
 
-	return snprintf(buf, PAGE_SIZE, "Serial Number: %u\n",
+	return sysfs_emit(buf, "Serial Number: %u\n",
 			mhi_cntrl->serial_number);
 }
 static DEVICE_ATTR_RO(serial_number);
-- 
2.17.1

Re: [PATCH] bus: mhi: core: replace snprintf with sysfs_emit

[Greg Kroah-Hartman]

On Wed, Dec 08, 2021 at 12:07:53AM -0800, Yihao Han wrote:
> coccinelle report:
> ./drivers/bus/mhi/core/init.c:97:8-16:
> WARNING: use scnprintf or sprintf
> Use sysfs_emit instead of scnprintf or sprintf makes more sense.
> 
> Signed-off-by: Yihao Han <hanyihao@vivo.com>
> ---
>  drivers/bus/mhi/core/init.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
> index 5aaca6d0f52b..a5a5c722731e 100644
> --- a/drivers/bus/mhi/core/init.c
> +++ b/drivers/bus/mhi/core/init.c
> @@ -94,7 +94,7 @@ static ssize_t serial_number_show(struct device *dev,
>  	struct mhi_device *mhi_dev = to_mhi_device(dev);
>  	struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
>  
> -	return snprintf(buf, PAGE_SIZE, "Serial Number: %u\n",
> +	return sysfs_emit(buf, "Serial Number: %u\n",
>  			mhi_cntrl->serial_number);
>  }
>  static DEVICE_ATTR_RO(serial_number);
> -- 
> 2.17.1
> 

Why are you only changing one function in this file?  If you realyl want
to make a change like this, fix ALL sysfs show functions.

thanks,

greg k-h

Re: [PATCH] bus: mhi: core: replace snprintf with sysfs_emit

[Yihao Han]

On 2021/12/8 16:14, Greg Kroah-Hartman wrote:
> On Wed, Dec 08, 2021 at 12:07:53AM -0800, Yihao Han wrote:
>> coccinelle report:
>> ./drivers/bus/mhi/core/init.c:97:8-16:
>> WARNING: use scnprintf or sprintf
>> Use sysfs_emit instead of scnprintf or sprintf makes more sense.
>>
>> Signed-off-by: Yihao Han <hanyihao@vivo.com>
>> ---
>>   drivers/bus/mhi/core/init.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
>> index 5aaca6d0f52b..a5a5c722731e 100644
>> --- a/drivers/bus/mhi/core/init.c
>> +++ b/drivers/bus/mhi/core/init.c
>> @@ -94,7 +94,7 @@ static ssize_t serial_number_show(struct device *dev,
>>   	struct mhi_device *mhi_dev = to_mhi_device(dev);
>>   	struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
>>   
>> -	return snprintf(buf, PAGE_SIZE, "Serial Number: %u\n",
>> +	return sysfs_emit(buf, "Serial Number: %u\n",
>>   			mhi_cntrl->serial_number);
>>   }
>>   static DEVICE_ATTR_RO(serial_number);
>> -- 
>> 2.17.1
>>
> 
> Why are you only changing one function in this file?  If you realyl want
> to make a change like this, fix ALL sysfs show functions.
> 
> thanks,
> 
> greg k-h
Because "sysfs_emit()" is "scnprintf()" equivalent with "size" parameter 
equals to PAGE_SIZE,So sysfs_emit can only be used here in this file. 
And "scnprintf" is better than "snprintf" because the former returns 
number of characters written to "buf". So I think we can use 
"sysfs_emit()" instead of "snprintf()".

Thanks,
Yihao

Re: [PATCH] bus: mhi: core: replace snprintf with sysfs_emit

[Greg Kroah-Hartman]

On Wed, Dec 08, 2021 at 10:03:30PM +0800, Yihao Han wrote:
> 
> 
> On 2021/12/8 16:14, Greg Kroah-Hartman wrote:
> > On Wed, Dec 08, 2021 at 12:07:53AM -0800, Yihao Han wrote:
> > > coccinelle report:
> > > ./drivers/bus/mhi/core/init.c:97:8-16:
> > > WARNING: use scnprintf or sprintf
> > > Use sysfs_emit instead of scnprintf or sprintf makes more sense.
> > > 
> > > Signed-off-by: Yihao Han <hanyihao@vivo.com>
> > > ---
> > >   drivers/bus/mhi/core/init.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
> > > index 5aaca6d0f52b..a5a5c722731e 100644
> > > --- a/drivers/bus/mhi/core/init.c
> > > +++ b/drivers/bus/mhi/core/init.c
> > > @@ -94,7 +94,7 @@ static ssize_t serial_number_show(struct device *dev,
> > >   	struct mhi_device *mhi_dev = to_mhi_device(dev);
> > >   	struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
> > > -	return snprintf(buf, PAGE_SIZE, "Serial Number: %u\n",
> > > +	return sysfs_emit(buf, "Serial Number: %u\n",
> > >   			mhi_cntrl->serial_number);
> > >   }
> > >   static DEVICE_ATTR_RO(serial_number);
> > > -- 
> > > 2.17.1
> > > 
> > 
> > Why are you only changing one function in this file?  If you realyl want
> > to make a change like this, fix ALL sysfs show functions.
> > 
> > thanks,
> > 
> > greg k-h
> Because "sysfs_emit()" is "scnprintf()" equivalent with "size" parameter
> equals to PAGE_SIZE,So sysfs_emit can only be used here in this file. And
> "scnprintf" is better than "snprintf" because the former returns number of
> characters written to "buf". So I think we can use "sysfs_emit()" instead of
> "snprintf()".

I am not disagreeing with changing this at all that is is incorrect.  I
am complaining that you are only changing ONE of the functions in this
file and not ALL of them.  If this type of change is so important to
make, please do it for all places it is needed in the file, and in all
of the drivers/bus/mhi/ code.

thanks,

greg k-h