| Message ID | 20230628153342.53406-1-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v1,1/1] kasan: Replace strreplace() with strchrnul() | expand |
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'; ?
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.
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)
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...
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.
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
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);
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(-)