Re: [PATCH] strtok -> strsep (The Easy Cases)

[Rene Scharfe <l.s.r@web.de>]

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é