[PATCH v1 1/1] kasan: Replace strreplace() with strchrnul()

[PATCH v1 1/1] kasan: Replace strreplace() with strchrnul()

[Andy Shevchenko]

We don't need to traverse over the entire string and replace
occurrences of a character with '\0'. The first match will
suffice. Hence, replace strreplace() with strchrnul().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 mm/kasan/report_generic.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/mm/kasan/report_generic.c b/mm/kasan/report_generic.c
index 51a1e8a8877f..63a34eac4a8c 100644
--- a/mm/kasan/report_generic.c
+++ b/mm/kasan/report_generic.c
@@ -264,6 +264,7 @@ static void print_decoded_frame_descr(const char *frame_descr)
 	while (num_objects--) {
 		unsigned long offset;
 		unsigned long size;
+		char *p;
 
 		/* access offset */
 		if (!tokenize_frame_descr(&frame_descr, token, sizeof(token),
@@ -282,7 +283,7 @@ static void print_decoded_frame_descr(const char *frame_descr)
 			return;
 
 		/* Strip line number; without filename it's not very helpful. */
-		strreplace(token, ':', '\0');
+		p[strchrnul(token, ':') - token] = '\0';
 
 		/* Finally, print object information. */
 		pr_err(" [%lu, %lu) '%s'", offset, offset + size, token);
-- 
2.40.0.1.gaa8946217a0b

Re: [PATCH v1 1/1] kasan: Replace strreplace() with strchrnul()

[Alexander Potapenko]

On Wed, Jun 28, 2023 at 5:34 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> We don't need to traverse over the entire string and replace
> occurrences of a character with '\0'. The first match will
> suffice. Hence, replace strreplace() with strchrnul().
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  mm/kasan/report_generic.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/mm/kasan/report_generic.c b/mm/kasan/report_generic.c
> index 51a1e8a8877f..63a34eac4a8c 100644
> --- a/mm/kasan/report_generic.c
> +++ b/mm/kasan/report_generic.c
> @@ -264,6 +264,7 @@ static void print_decoded_frame_descr(const char *frame_descr)
>         while (num_objects--) {
>                 unsigned long offset;
>                 unsigned long size;
> +               char *p;
>
>                 /* access offset */
>                 if (!tokenize_frame_descr(&frame_descr, token, sizeof(token),
> @@ -282,7 +283,7 @@ static void print_decoded_frame_descr(const char *frame_descr)
>                         return;
>
>                 /* Strip line number; without filename it's not very helpful. */
> -               strreplace(token, ':', '\0');
> +               p[strchrnul(token, ':') - token] = '\0';

Why not just
   *(strchrnul(token, ':')) = '\0';
?

Re: [PATCH v1 1/1] kasan: Replace strreplace() with strchrnul()

[Andy Shevchenko]

On Wed, Jun 28, 2023 at 05:39:26PM +0200, Alexander Potapenko wrote:
> On Wed, Jun 28, 2023 at 5:34 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:

...

> >                 /* Strip line number; without filename it's not very helpful. */
> > -               strreplace(token, ':', '\0');
> > +               p[strchrnul(token, ':') - token] = '\0';
> 
> Why not just
>    *(strchrnul(token, ':')) = '\0';
> ?

I don't like Pythonish style in the C. But if you insist, I can update it.

-- 
With Best Regards,
Andy Shevchenko

RE: [PATCH v1 1/1] kasan: Replace strreplace() with strchrnul()

[David Laight]

From: Andy Shevchenko
> Sent: 28 June 2023 16:34
> 
> We don't need to traverse over the entire string and replace
> occurrences of a character with '\0'. The first match will
> suffice. Hence, replace strreplace() with strchrnul().
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  mm/kasan/report_generic.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/kasan/report_generic.c b/mm/kasan/report_generic.c
> index 51a1e8a8877f..63a34eac4a8c 100644
> --- a/mm/kasan/report_generic.c
> +++ b/mm/kasan/report_generic.c
> @@ -264,6 +264,7 @@ static void print_decoded_frame_descr(const char *frame_descr)
>  	while (num_objects--) {
>  		unsigned long offset;
>  		unsigned long size;
> +		char *p;
> 
>  		/* access offset */
>  		if (!tokenize_frame_descr(&frame_descr, token, sizeof(token),
> @@ -282,7 +283,7 @@ static void print_decoded_frame_descr(const char *frame_descr)
>  			return;
> 
>  		/* Strip line number; without filename it's not very helpful. */
> -		strreplace(token, ':', '\0');
> +		p[strchrnul(token, ':') - token] = '\0';

Isn't 'p' undefined here?

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH v1 1/1] kasan: Replace strreplace() with strchrnul()

['Andy Shevchenko']

On Thu, Jun 29, 2023 at 02:32:13PM +0000, David Laight wrote:
> From: Andy Shevchenko
> > Sent: 28 June 2023 16:34

...

> >  		/* Strip line number; without filename it's not very helpful. */
> > -		strreplace(token, ':', '\0');
> > +		p[strchrnul(token, ':') - token] = '\0';
> 
> Isn't 'p' undefined here?

Yep, should be token. Not sure what I was thinking about...

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/1] kasan: Replace strreplace() with strchrnul()

[Andy Shevchenko]

On Wed, Jun 28, 2023 at 06:33:42PM +0300, Andy Shevchenko wrote:
> We don't need to traverse over the entire string and replace
> occurrences of a character with '\0'. The first match will
> suffice. Hence, replace strreplace() with strchrnul().

Not that it's a hot path, the bloat-o-meter shows +6 bytes on x86_64,
the change seems has no added value, self-rejected.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/1] kasan: Replace strreplace() with strchrnul()

[kernel test robot]

Hi Andy,

kernel test robot noticed the following build warnings:

[auto build test WARNING on akpm-mm/mm-everything]
[also build test WARNING on linus/master v6.5-rc2 next-20230718]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Andy-Shevchenko/kasan-Replace-strreplace-with-strchrnul/20230628-233727
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20230628153342.53406-1-andriy.shevchenko%40linux.intel.com
patch subject: [PATCH v1 1/1] kasan: Replace strreplace() with strchrnul()
config: x86_64-randconfig-x001-20230718 (https://download.01.org/0day-ci/archive/20230719/202307191350.tJh2PZdE-lkp@intel.com/config)
compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project.git 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
reproduce: (https://download.01.org/0day-ci/archive/20230719/202307191350.tJh2PZdE-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202307191350.tJh2PZdE-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> mm/kasan/report_generic.c:286:3: warning: variable 'p' is uninitialized when used here [-Wuninitialized]
                   p[strchrnul(token, ':') - token] = '\0';
                   ^
   mm/kasan/report_generic.c:267:10: note: initialize the variable 'p' to silence this warning
                   char *p;
                          ^
                           = NULL
   1 warning generated.


vim +/p +286 mm/kasan/report_generic.c

   242	
   243	static void print_decoded_frame_descr(const char *frame_descr)
   244	{
   245		/*
   246		 * We need to parse the following string:
   247		 *    "n alloc_1 alloc_2 ... alloc_n"
   248		 * where alloc_i looks like
   249		 *    "offset size len name"
   250		 * or "offset size len name:line".
   251		 */
   252	
   253		char token[64];
   254		unsigned long num_objects;
   255	
   256		if (!tokenize_frame_descr(&frame_descr, token, sizeof(token),
   257					  &num_objects))
   258			return;
   259	
   260		pr_err("\n");
   261		pr_err("This frame has %lu %s:\n", num_objects,
   262		       num_objects == 1 ? "object" : "objects");
   263	
   264		while (num_objects--) {
   265			unsigned long offset;
   266			unsigned long size;
   267			char *p;
   268	
   269			/* access offset */
   270			if (!tokenize_frame_descr(&frame_descr, token, sizeof(token),
   271						  &offset))
   272				return;
   273			/* access size */
   274			if (!tokenize_frame_descr(&frame_descr, token, sizeof(token),
   275						  &size))
   276				return;
   277			/* name length (unused) */
   278			if (!tokenize_frame_descr(&frame_descr, NULL, 0, NULL))
   279				return;
   280			/* object name */
   281			if (!tokenize_frame_descr(&frame_descr, token, sizeof(token),
   282						  NULL))
   283				return;
   284	
   285			/* Strip line number; without filename it's not very helpful. */
 > 286			p[strchrnul(token, ':') - token] = '\0';
   287	
   288			/* Finally, print object information. */
   289			pr_err(" [%lu, %lu) '%s'", offset, offset + size, token);
   290		}
   291	}
   292	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki