106372 - Need PR_strtok_r()

Status: RESOLVED FIXED

(NSPR :: NSPR, enhancement, P1)

Description

[Roland Mainz]

We _urgendly_ need a NSPR version of strtok_r().

Some code already uses strtok_r() and strtok().
But the first one is not available on all platforms we support (per cls) and the
2nd one (strtok()) should be avoided in multithreaded apps. as it is not
thread-safe on all platforms.

Patch in bug 100069 has a version of strtok_r(). Just grab it and rename it to
PR_strtok_r() ... :-)

Comment 1

[hacker formerly known as [e-mail]]

I just found out today that xpcom/ds/nsCRT.cpp has a supposedly threadsafe
version of strtok.  You might want to try it.

Comment 2

[Roland Mainz]

1. |strtok()| does not help because I use strtok_r() in a "nested way"
(|for|-loop using strtok_r() which calls function which itself uses strtok_r()).
|strtok()| won't help... see [3]
2. The function sits in XPCOM, not in NSPR. I think it would be very usefull to
have POSIX-like |strtok_r()| in NSPR (use own if OS does not provide one - but
use the OS version if present).
3. It looks that xpcom/ds/nsCRT.cpp#nsCRT::strtok() is a completly different
beast than POSIX strtok_r() (just looking at comments like "Unlike regular
strtok, the first argument cannot be null." and "too many delimiters"... ewww...
;-( ).

Comment 3

[Roland Mainz]

Stealing this from wtc...

Comment 4

[Roland Mainz]

Patch follows in a few mins ...

Comment 5

[Wan-Teh Chang]

This should be added to mozilla/nsprpub/lib/libc.
The function should use the PL_ prefix, that is,
it should be renamed PL_strtok_r().

If PL_strtok_r calls strtok_r where strtok_r is
present, PL_strtok_r may need to have the same
signature as strtok_r (in parcular, with respect
to the use of const) for the best performance.

char *strtok_r(char *s, const char *sep, char **lasts);

Comment 6

[Roland Mainz]

Attachment: Patch for 2001-10-20-08-trunk

Comment 7

[Roland Mainz]

> This should be added to mozilla/nsprpub/lib/libc.
> The function should use the PL_ prefix, that is,
> it should be renamed PL_strtok_r().

Done, see patch.

> If PL_strtok_r calls strtok_r where strtok_r is
> present, PL_strtok_r may need to have the same
> signature as strtok_r (in parcular, with respect
> to the use of const) for the best performance

I would prefer a simple |#define PL_foobar(arg1, arg2) foobar(arg1, arg2)| to
save the function call overhead (for example I _hate_ X.org code which wastes a
lot of time in such wrapper code).
However, the whole "use native function if OS has it" can be done in a later
step...

----

Requesting r=/sr= ...

Comment 8

[Daniel (Leaf) Nunes]

cc'ing a potential nspr reviewer.

Comment 9

[Roland Mainz]

cc'ing some potential nspr reviewers.

Comment 10

[Wan-Teh Chang]

Attachment: Roland Mainz's patch, edited, added comments and a test.

Comment 11

[Wan-Teh Chang]

Roland, I edited your patch to indent by four spaces,
avoid the goto statement, and use for loops.  I also
added comments in plstr.h to document the new function
and added a test case for PL_strtok_r.

Comment 12

[Roland Mainz]

Comment on attachment 55868 [details] [diff] [review]
Roland Mainz's patch, edited, added comments and a test.

Thanks for the new patch... :-)

r=[e-mail]

Comment 13

[Roland Mainz]

wtc:
Wanna check this in for me, please ?
I do not have CVS checkin permission ....

Comment 14

[Wan-Teh Chang]

Roland, I will try to do that later today.
This bug won't be marked fixed until I find
someone to help me add the new file to the
Mac project file.

Comment 15

[Wan-Teh Chang]

I checked in my patch (with minor editing) on the tip
of NSPRPUB_PRE_4_2_CLIENT_BRANCH of NSPR.

Simon, could you please add mozilla/nsprpub/lib/libc/src/strtok.c
to NSPR's Mac project file on the NSPRPUB_PRE_4_2_CLIENT_BRANCH?

Comment 16

[Wan-Teh Chang]

Sorry, I meant to say
    I checked in my patch (with minor editing) on the tip
    and the NSPRPUB_PRE_4_2_CLIENT_BRANCH of NSPR.
    ^^^

Comment 17

[Simon Fraser [no longer active]]

Done. So I should do the mac project on the NSPR tip too?

Comment 18

[Wan-Teh Chang]

Simon Fraser wrote:
> So I should do the mac project on the NSPR tip too?

Yes, please.  I did not dare to ask because I could
copy the file to the NSPR tip and check it in.

Thanks! :-)

Comment 19

[Simon Fraser [no longer active]]

Mac project fixed on the NSPR tip.

Comment 20

[Wan-Teh Chang]

Marked the bug fixed.

Comment 21

[Roland Mainz]

Filed bug 109177 as a follow-up to kill the |strtok()|-clones from the tree and
replace them all with |PL_strtok_r()| ...