Re: [Patch] fix unsigned long fast_strtoul_10(char **endptr) for Linux 2.4

2012-02-28 Thread Ralf Friedl

walter harms wrote:

Enabling the fast proc scan code for linux2.4 causes top
to display strange numbers [see:possible bug in proc fast scan code]

the reason for this is that the last char in proc/$$/stat is \n.
This is no problem for  2.4 since the stats line contains additional chars.

Please be aware that the current code does NOT stop at \0 !  adding more
tokens to read will cause the code to read beyond \0 !

i also killed the variable c what saves 1 byte.
  
This variable would live on the stack if it would ever leave the 
register (which it doesn't, even on a x86 with very few registers).


Independent of this patch, you do realize that initializing n to 
*str-'0' is some kind of manual loop unrolling? Both of the following 
versions are smaller, at the cost of one more pass through the loop. I 
don't know whether it is called often enough for this to matter.
The first version is smaller with -O9 -m32, -Os -m32 and -Os -m64 while 
it is larger with -O9 -m64 with GCC 4.6.2. In each case both versions 
are smaller than the original.


unsigned long fast_strtoul_10(char **endptr)
{
 char c;
 char *str = *endptr;
 unsigned long n = 0;

 while ((c = *str++)  ' ')
   n = n*10 + (c - '0');

 *endptr = str; /* We skip trailing space! */
 return n;
}

unsigned long fast_strtoul_10(char **endptr)
{
 char c;
 char *str = *endptr;
 unsigned long n = 0;

 while (*str  ' ')
   n = n*10 + (*str++ - '0');

 *endptr = str + 1; /* We skip trailing space! */
 return n;
}

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


Re: [Patch] fix unsigned long fast_strtoul_10(char **endptr) for Linux 2.4

2012-02-28 Thread walter harms


Am 28.02.2012 13:27, schrieb Ralf Friedl:
 walter harms wrote:
 Enabling the fast proc scan code for linux2.4 causes top
 to display strange numbers [see:possible bug in proc fast scan code]

 the reason for this is that the last char in proc/$$/stat is \n.
 This is no problem for  2.4 since the stats line contains additional
 chars.

 Please be aware that the current code does NOT stop at \0 !  adding more
 tokens to read will cause the code to read beyond \0 !

 i also killed the variable c what saves 1 byte.
   
 This variable would live on the stack if it would ever leave the
 register (which it doesn't, even on a x86 with very few registers).
 
 Independent of this patch, you do realize that initializing n to
 *str-'0' is some kind of manual loop unrolling? Both of the following
 versions are smaller, at the cost of one more pass through the loop. I
 don't know whether it is called often enough for this to matter.
 The first version is smaller with -O9 -m32, -Os -m32 and -Os -m64 while
 it is larger with -O9 -m64 with GCC 4.6.2. In each case both versions
 are smaller than the original.
 
 unsigned long fast_strtoul_10(char **endptr)
 {
  char c;
  char *str = *endptr;
  unsigned long n = 0;
 
  while ((c = *str++)  ' ')
n = n*10 + (c - '0');
 
  *endptr = str; /* We skip trailing space! */
  return n;
 }
 
 unsigned long fast_strtoul_10(char **endptr)
 {
  char c;
  char *str = *endptr;
  unsigned long n = 0;
 
  while (*str  ' ')
n = n*10 + (*str++ - '0');
 
  *endptr = str + 1; /* We skip trailing space! */
  return n;
 }
 
 


Hello Ralf,
the current version has a major problem when you consider this: 0\0
the patch ( change != into  ) will only catch the current bug
(reading the last 0 wrong what actually happens on 2.4) but when the main
code reads again (perhaps to get additional information) it will read
beyond \0.
char *cp=0\n;
 fast_strtoul_10(cp); -- will work fine
 fast_strtoul_10(cp); -- will break

re
 wh

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


Re: [Patch] fix unsigned long fast_strtoul_10(char **endptr) for Linux 2.4

2012-02-28 Thread walter harms


Am 28.02.2012 17:08, schrieb Denys Vlasenko:
 On Tue, Feb 28, 2012 at 5:04 PM, walter harms wha...@bfs.de wrote:
 Hello Ralf,
 the current version has a major problem when you consider this: 0\0
 the patch ( change != into  ) will only catch the current bug
 (reading the last 0 wrong what actually happens on 2.4) but when the main
 code reads again (perhaps to get additional information) it will read
 beyond \0.
 char *cp=0\n;
  fast_strtoul_10(cp); -- will work fine
  fast_strtoul_10(cp); -- will break
 
 The whole point of fast_strtoul_10 is that it _knows_ that /proc data
 it is fed with is well-formatted. That's why it doesn't do a lot of error
 checking, for example: it simply assumes all chars  ' ' will be
 decimal digits.
 
 The thing you are pointing out is just another case of lack of error check:
 fast_strtoul_10() is not supposed to be called when you are unsure that
 field you are trying to parse actually exists.
 

i noticed that also, the difference for me is that ignoring \0 may cause an
segfault, while taking \n for a number will result in bad data.

re,
 wh
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


[Patch] fix unsigned long fast_strtoul_10(char **endptr) for Linux 2.4

2012-02-27 Thread walter harms
Hi list,
Enabling the fast proc scan code for linux2.4 causes top
to display strange numbers [see:possible bug in proc fast scan code]

the reason for this is that the last char in proc/$$/stat is \n.
This is no problem for  2.4 since the stats line contains additional chars.

Please be aware that the current code does NOT stop at \0 !  adding more
tokens to read will cause the code to read beyond \0 !

i also killed the variable c what saves 1 byte.

re,
 wh

Signed-off-by: wharms wha...@bfs.de

--- procps.c.org2012-02-20 19:22:58.0 +0100
+++ procps.c2012-02-27 18:45:11.0 +0100
@@ -143,12 +143,11 @@
 /* We cut a lot of corners here for speed */
 static unsigned long fast_strtoul_10(char **endptr)
 {
-   char c;
char *str = *endptr;
unsigned long n = *str - '0';

-   while ((c = *++str) != ' ')
-   n = n*10 + (c - '0');
+   while ( *str  ' ')
+   n = n*10 + ( *str++ - '0');

*endptr = str + 1; /* We skip trailing space! */
return n;
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


Re: [Patch] fix unsigned long fast_strtoul_10(char **endptr) for Linux 2.4

2012-02-27 Thread Denys Vlasenko
On Monday 27 February 2012 18:56, walter harms wrote:
 Hi list,
 Enabling the fast proc scan code for linux2.4 causes top
 to display strange numbers [see:possible bug in proc fast scan code]
 
 the reason for this is that the last char in proc/$$/stat is \n.
 This is no problem for  2.4 since the stats line contains additional chars.
 
 Please be aware that the current code does NOT stop at \0 !  adding more
 tokens to read will cause the code to read beyond \0 !

Thanks for diagnosing the problem!

 i also killed the variable c what saves 1 byte.

...and introduced a bug: you are using str[0] twice now.

I fixed the code in current git. Please try it.

-- 
vda


___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox