| Message ID | 20211208080816.43351-1-hanyihao@vivo.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | bus: mhi: core: replace snprintf with sysfs_emit | expand |
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
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
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
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);
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(-)