Re: [Patch] fix unsigned long fast_strtoul_10(char **endptr) for Linux 2.4
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
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
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
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
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