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 <[email protected]>
---
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
On Wed, Jun 28, 2023 at 5:34 PM Andy Shevchenko
<[email protected]> 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 <[email protected]>
> ---
> 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';
?
On Wed, Jun 28, 2023 at 05:39:26PM +0200, Alexander Potapenko wrote:
> On Wed, Jun 28, 2023 at 5:34 PM Andy Shevchenko
> <[email protected]> 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
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 <[email protected]>
> ---
> 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)
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
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
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/[email protected]/config)
compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project.git 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
reproduce: (https://download.01.org/0day-ci/archive/20230719/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
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