From: Jing Yao <[email protected]>
Reported-by: Zeal Robot <[email protected]>
Signed-off-by: Jing Yao <[email protected]>
---
drivers/tty/serial/8250/8250_port.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 5775cbff8f6e..557e8b13b5c1 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -3099,7 +3099,7 @@ static ssize_t rx_trig_bytes_show(struct device *dev,
if (rxtrig_bytes < 0)
return rxtrig_bytes;
- return snprintf(buf, PAGE_SIZE, "%d\n", rxtrig_bytes);
+ return sysfs_emit(buf, PAGE_SIZE, "%d\n", rxtrig_bytes);
}
static int do_set_rxtrig(struct tty_port *port, unsigned char bytes)
--
2.25.1
On Thu, Oct 28, 2021 at 10:13:50AM +0000, [email protected] wrote:
> From: Jing Yao <[email protected]>
Where's the commit message?
Also, look at the log for the driver you're changing for the Subject
prefix you should use. Including "drivers:" is never right.
> Reported-by: Zeal Robot <[email protected]>
> Signed-off-by: Jing Yao <[email protected]>
> ---
> drivers/tty/serial/8250/8250_port.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index 5775cbff8f6e..557e8b13b5c1 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -3099,7 +3099,7 @@ static ssize_t rx_trig_bytes_show(struct device *dev,
> if (rxtrig_bytes < 0)
> return rxtrig_bytes;
>
> - return snprintf(buf, PAGE_SIZE, "%d\n", rxtrig_bytes);
> + return sysfs_emit(buf, PAGE_SIZE, "%d\n", rxtrig_bytes);
FFS... This would not even compile, at least not without a warning, as
it's completely broken.
You do know that you have to at least compile-test your patches, right?
> }
>
> static int do_set_rxtrig(struct tty_port *port, unsigned char bytes)
Johan
On Thu, 28 Oct 2021, Johan Hovold wrote:
> Where's the commit message?
Or to put it explicitly: please always include a full description of any
change submitted, where you explain what your change is needed for and why
you have made it like this.
Please refer to Documentation/process/submitting-patches.rst in the
source tree for a full recipe for submitting patches, which says, among
others:
"[...] The text should be written in such detail so that when read
weeks, months or even years later, it can give the reader the needed
details to grasp the reasoning for **why** the patch was created."
Thanks,
Maciej
Hi,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on tty/tty-testing]
[also build test WARNING on staging/staging-testing v5.15-rc7 next-20211028]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/cgel-zte-gmail-com/drivers-tty-replace-snprintf-in-show-functions-with-sysfs_emit/20211028-181624
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
config: ia64-defconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/03b103ac2507a7958ac9e45fe6a53110e1140625
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review cgel-zte-gmail-com/drivers-tty-replace-snprintf-in-show-functions-with-sysfs_emit/20211028-181624
git checkout 03b103ac2507a7958ac9e45fe6a53110e1140625
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=ia64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
In file included from arch/ia64/include/asm/ptrace.h:46,
from arch/ia64/include/asm/processor.h:20,
from arch/ia64/include/asm/timex.h:15,
from include/linux/timex.h:65,
from include/linux/time32.h:13,
from include/linux/time.h:60,
from include/linux/stat.h:19,
from include/linux/module.h:13,
from drivers/tty/serial/8250/8250_port.c:14:
drivers/tty/serial/8250/8250_port.c: In function 'rx_trig_bytes_show':
>> arch/ia64/include/asm/page.h:42:33: warning: passing argument 2 of 'sysfs_emit' makes pointer from integer without a cast [-Wint-conversion]
42 | #define PAGE_SIZE (__IA64_UL_CONST(1) << PAGE_SHIFT)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| |
| long unsigned int
drivers/tty/serial/8250/8250_port.c:3102:32: note: in expansion of macro 'PAGE_SIZE'
3102 | return sysfs_emit(buf, PAGE_SIZE, "%d\n", rxtrig_bytes);
| ^~~~~~~~~
In file included from include/linux/kobject.h:20,
from include/linux/module.h:21,
from drivers/tty/serial/8250/8250_port.c:14:
include/linux/sysfs.h:341:39: note: expected 'const char *' but argument is of type 'long unsigned int'
341 | int sysfs_emit(char *buf, const char *fmt, ...);
| ~~~~~~~~~~~~^~~
vim +/sysfs_emit +42 arch/ia64/include/asm/page.h
^1da177e4c3f415 include/asm-ia64/page.h Linus Torvalds 2005-04-16 41
^1da177e4c3f415 include/asm-ia64/page.h Linus Torvalds 2005-04-16 @42 #define PAGE_SIZE (__IA64_UL_CONST(1) << PAGE_SHIFT)
^1da177e4c3f415 include/asm-ia64/page.h Linus Torvalds 2005-04-16 43 #define PAGE_MASK (~(PAGE_SIZE - 1))
^1da177e4c3f415 include/asm-ia64/page.h Linus Torvalds 2005-04-16 44
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
Hi,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on tty/tty-testing]
[also build test ERROR on staging/staging-testing v5.15-rc7 next-20211028]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/cgel-zte-gmail-com/drivers-tty-replace-snprintf-in-show-functions-with-sysfs_emit/20211028-181624
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
config: microblaze-buildonly-randconfig-r002-20211028 (attached as .config)
compiler: microblaze-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/03b103ac2507a7958ac9e45fe6a53110e1140625
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review cgel-zte-gmail-com/drivers-tty-replace-snprintf-in-show-functions-with-sysfs_emit/20211028-181624
git checkout 03b103ac2507a7958ac9e45fe6a53110e1140625
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=microblaze
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
In file included from include/linux/mm_types_task.h:16,
from include/linux/mm_types.h:5,
from include/linux/buildid.h:5,
from include/linux/module.h:14,
from drivers/tty/serial/8250/8250_port.c:14:
drivers/tty/serial/8250/8250_port.c: In function 'rx_trig_bytes_show':
>> arch/microblaze/include/asm/page.h:24:25: error: passing argument 2 of 'sysfs_emit' makes pointer from integer without a cast [-Werror=int-conversion]
24 | #define PAGE_SIZE (ASM_CONST(1) << PAGE_SHIFT)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
| |
| long unsigned int
drivers/tty/serial/8250/8250_port.c:3102:32: note: in expansion of macro 'PAGE_SIZE'
3102 | return sysfs_emit(buf, PAGE_SIZE, "%d\n", rxtrig_bytes);
| ^~~~~~~~~
In file included from include/linux/kobject.h:20,
from include/linux/module.h:21,
from drivers/tty/serial/8250/8250_port.c:14:
include/linux/sysfs.h:341:39: note: expected 'const char *' but argument is of type 'long unsigned int'
341 | int sysfs_emit(char *buf, const char *fmt, ...);
| ~~~~~~~~~~~~^~~
cc1: all warnings being treated as errors
vim +/sysfs_emit +24 arch/microblaze/include/asm/page.h
4b87d7a4f91d31 Michal Simek 2009-03-27 21
4b87d7a4f91d31 Michal Simek 2009-03-27 22 /* PAGE_SHIFT determines the page size */
ba9c4f88d74783 Steven J. Magnani 2010-05-13 23 #define PAGE_SHIFT 12
ba9c4f88d74783 Steven J. Magnani 2010-05-13 @24 #define PAGE_SIZE (ASM_CONST(1) << PAGE_SHIFT)
4b87d7a4f91d31 Michal Simek 2009-03-27 25 #define PAGE_MASK (~(PAGE_SIZE-1))
4b87d7a4f91d31 Michal Simek 2009-03-27 26
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
On Thu, Oct 28, 2021 at 10:13:50AM +0000, [email protected] wrote:
> From: Jing Yao <[email protected]>
>
> Reported-by: Zeal Robot <[email protected]>
Please fix your broken "robot" before submitting any more patches to the
Linux kernel project.
good luck!
greg k-h
On Fri, 2021-10-29 at 07:44 +0200, Greg KH wrote:
> On Thu, Oct 28, 2021 at 10:13:50AM +0000, [email protected] wrote:
> > From: Jing Yao <[email protected]>
> >
> > Reported-by: Zeal Robot <[email protected]>
>
> Please fix your broken "robot" before submitting any more patches to the
> Linux kernel project.
There's no indication the robot and the patch author/submitter are
the same.
I think the "robot" _reporting_ the issue is reasonable, but not the
patch author/submitter.
Though whatever the "Zeal Robot" is should likely be public otherwise
the Reported-by: reference isn't useful.
From: Jing Yao <[email protected]>
coccicheck complains about the use of snprintf() in sysfs show
functions:
WARNING use scnprintf or sprintf
Use sysfs_emit instead of scnprintf or sprintf makes more sense.
Reported-by: Zeal Robot <[email protected]>
Signed-off-by: Jing Yao <[email protected]>
---
drivers/tty/serial/8250/8250_port.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 5775cbff8f6e..3d58f383152e 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -3099,7 +3099,7 @@ static ssize_t rx_trig_bytes_show(struct device *dev,
if (rxtrig_bytes < 0)
return rxtrig_bytes;
- return snprintf(buf, PAGE_SIZE, "%d\n", rxtrig_bytes);
+ return sysfs_emit(buf, "%d\n", rxtrig_bytes);
}
static int do_set_rxtrig(struct tty_port *port, unsigned char bytes)
--
2.25.1
On Tue, Nov 02, 2021 at 06:52:06AM +0000, [email protected] wrote:
> From: Jing Yao <[email protected]>
>
> coccicheck complains about the use of snprintf() in sysfs show
> functions:
> WARNING use scnprintf or sprintf
>
> Use sysfs_emit instead of scnprintf or sprintf makes more sense.
>
> Reported-by: Zeal Robot <[email protected]>
> Signed-off-by: Jing Yao <[email protected]>
> ---
What changed since v1? Always include a changelog here.
Also you ignored my comment on the patch Subject. Including "drivers: "
is never correct. You should also mention which driver you're changing
since it's not done subtree-wide for tty.
Again, look at the git log for the driver you're changing. In this case
it should have been something like:
serial: 8250: use sysfs_emit...
> drivers/tty/serial/8250/8250_port.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index 5775cbff8f6e..3d58f383152e 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -3099,7 +3099,7 @@ static ssize_t rx_trig_bytes_show(struct device *dev,
> if (rxtrig_bytes < 0)
> return rxtrig_bytes;
>
> - return snprintf(buf, PAGE_SIZE, "%d\n", rxtrig_bytes);
> + return sysfs_emit(buf, "%d\n", rxtrig_bytes);
> }
>
> static int do_set_rxtrig(struct tty_port *port, unsigned char bytes)
Johan
From: Jing Yao <[email protected]>
coccicheck complains about the use of snprintf() in sysfs show
functions:
WARNING use scnprintf or sprintf
Use sysfs_emit instead of scnprintf or sprintf makes more sense.
Reported-by: Zeal Robot <[email protected]>
Signed-off-by: Jing Yao <[email protected]>
---
Changes since v1 & v2:
- Remove excess and wrong parameter 'PAGE_SIZE' in sysfs_emit function.
- Revise the wrong patch Subject.
drivers/tty/serial/8250/8250_port.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 5775cbff8f6e..3d58f383152e 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -3099,7 +3099,7 @@ static ssize_t rx_trig_bytes_show(struct device *dev,
if (rxtrig_bytes < 0)
return rxtrig_bytes;
- return snprintf(buf, PAGE_SIZE, "%d\n", rxtrig_bytes);
+ return sysfs_emit(buf, "%d\n", rxtrig_bytes);
}
static int do_set_rxtrig(struct tty_port *port, unsigned char bytes)
--
2.25.1
On Thu, Nov 04, 2021 at 11:47:54AM +0000, [email protected] wrote:
> From: Jing Yao <[email protected]>
>
> coccicheck complains about the use of snprintf() in sysfs show
> functions:
> WARNING use scnprintf or sprintf
>
> Use sysfs_emit instead of scnprintf or sprintf makes more sense.
>
> Reported-by: Zeal Robot <[email protected]>
> Signed-off-by: Jing Yao <[email protected]>
> ---
>
> Changes since v1 & v2:
> - Remove excess and wrong parameter 'PAGE_SIZE' in sysfs_emit function.
> - Revise the wrong patch Subject.
Looks like there are a few more cases in
drivers/tty/serial/8250/8250_aspeed_vuart.c
which you could convert in a follow up patch.
> drivers/tty/serial/8250/8250_port.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index 5775cbff8f6e..3d58f383152e 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -3099,7 +3099,7 @@ static ssize_t rx_trig_bytes_show(struct device *dev,
> if (rxtrig_bytes < 0)
> return rxtrig_bytes;
>
> - return snprintf(buf, PAGE_SIZE, "%d\n", rxtrig_bytes);
> + return sysfs_emit(buf, "%d\n", rxtrig_bytes);
> }
>
> static int do_set_rxtrig(struct tty_port *port, unsigned char bytes)
This one looks good as is now:
Reviewed-by: Johan Hovold <[email protected]>
Johan