Re: [PATCH] strtok -> strsep (The Easy Cases)
Rusty Russell <[EMAIL PROTECTED]> schrieb am 04.05.01: > In message <01050413055100.00907@golmepha> you write: > > Am Freitag, 4. Mai 2001 02:57 schrieb Rusty Russell: > > > There are two cases where the substitution is problematic: > > > > Yes, but... > > > > The cases which my patch modifies are of a different kind: > > The very first hunk of your patch is wrong. I didn't check the > others. Note that the declaration of switches is: Oops. *blush* I think all other chunks are OK, but I'll check that thoroughly when I'll be home again next monday. Thank you for taking the time to look at my patch. René (who bites the table out of shame) __ Ferienklick.de - 225 Reisekataloge auf einen Blick! Direkt zu Ihrem Traumurlaub: http://ferienklick.de/?PP=2-0-100-105-0 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] strtok -> strsep (The Easy Cases)
In message <01050413055100.00907@golmepha> you write:
> Am Freitag, 4. Mai 2001 02:57 schrieb Rusty Russell:
> > There are two cases where the substitution is problematic:
>
> Yes, but...
>
> The cases which my patch modifies are of a different kind:
The very first hunk of your patch is wrong. I didn't check the
others. Note that the declaration of switches is:
char switches[len+1];
--- linux-2.4.4/arch/m68k/atari/config.cTue Nov 28 02:57:34 2000
+++ linux-2.4.4-rs/arch/m68k/atari/config.c Tue May 1 17:03:46 2001
@@ -206,13 +206,15 @@
/* parse the options */
-for( p = strtok( switches, "," ); p; p = strtok( NULL, "," ) ) {
+while( p = strsep( &switches, "," ) ) {
+ if (!*p)
+ continue;
ovsc_shift = 0;
if (strncmp( p, "ov_", 3 ) == 0) {
p += 3;
Cheers,
Rusty.
--
Premature optmztion is rt of all evl. --DK
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] strtok -> strsep (The Easy Cases)
Am Freitag, 4. Mai 2001 02:57 schrieb Rusty Russell:
> In message <01050120580701.01713@golmepha> you write:
> > Hello,
Hi!
> >
> > the patch at the bottom does the bulk job of strtok replacement. It's a
> > very boring patch, containing easy cases, only. It became a bit big, too,
> > but I trust you can digest it nevertheless. It's made against kernel
> > version 2.4.4.
>
> There are two cases where the substitution is problematic:
Yes, but...
The cases which my patch modifies are of a different kind:
int parse_options(char *options)
{
char *p;
/* for (p = strtok(options, ","); p; p = strtok(NULL, ",")) { */
while (p = strsep(&options, ",")) {
/* ... */
}
return 0;
}
No temporary array, no kfree(). Our variable "options" is used for strtsep,
only. Notice btw. how we destoy the string content to which "options"
points with both strtok and strsep.
That said, it's possible I made a stupid mistake, of course. Or two. Do
you agree on the correctness of the code above? If not, forget the whole
thing and I'll try again later.
>
> Array:
> char tmparray[500];
> strcpy(tmparray, str);
>
> /* for (p = strtok(tmparray, "n"); p; p = strtok(NULL, "n")) { */
> while ((p = strsep(&tmparray, ","))) {
>
> This is clearly wrong, and invokes a compiler warning. &tmparray ==
> tmparray (a cute C oddity I've never really liked). You are blowing
> away the first few characters in tmparray, and your parser won't work
> properly.
>
> Dynamic:
>
> char *tmp = strdup(str);
>
> /* for (p = strtok(tmp, "n"); p; p = strtok(NULL, "n")) { */
> while ((p = strsep(&tmp, ","))) {
> ...
> }
>
> kfree(tmp);
>
> Here, tmp has changed in the strsep implementation, and kfree will do
> bad things.
>
> There is a real reason to avoid strtok, and that is SMP and multple
> threads calling it at once (that said, I don't know of a problem yet).
> But this patch is a step backwards.
>
> Rusty.
> --
> Premature optmztion is rt of all evl. --DK
René
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] strtok -> strsep (The Easy Cases)
In message <01050120580701.01713@golmepha> you write:
> Hello,
>
> the patch at the bottom does the bulk job of strtok replacement. It's a
> very boring patch, containing easy cases, only. It became a bit big, too,
> but I trust you can digest it nevertheless. It's made against kernel
> version 2.4.4.
There are two cases where the substitution is problematic:
Array:
char tmparray[500];
strcpy(tmparray, str);
/* for (p = strtok(tmparray, "n"); p; p = strtok(NULL, "n")) { */
while ((p = strsep(&tmparray, ","))) {
This is clearly wrong, and invokes a compiler warning. &tmparray ==
tmparray (a cute C oddity I've never really liked). You are blowing
away the first few characters in tmparray, and your parser won't work
properly.
Dynamic:
char *tmp = strdup(str);
/* for (p = strtok(tmp, "n"); p; p = strtok(NULL, "n")) { */
while ((p = strsep(&tmp, ","))) {
...
}
kfree(tmp);
Here, tmp has changed in the strsep implementation, and kfree will do
bad things.
There is a real reason to avoid strtok, and that is SMP and multple
threads calling it at once (that said, I don't know of a problem yet).
But this patch is a step backwards.
Rusty.
--
Premature optmztion is rt of all evl. --DK
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
