This is the mail archive of the
systemtap@sourceware.org
mailing list for the systemtap project.
Re: [PATCH] Fix tokenize function and test.
- From: Josh Stone <jistone at redhat dot com>
- To: Przemyslaw Pawelczyk <przemyslaw at pawelczyk dot it>
- Cc: systemtap at sourceware dot org
- Date: Mon, 15 Jun 2009 13:00:50 -0700
- Subject: Re: [PATCH] Fix tokenize function and test.
- References: <1244863762.20494.8055@debian> <1244896709.407628.14220@debian>
On 06/13/2009 05:33 AM, Przemyslaw Pawelczyk wrote:
> Previous implementation was error-prone, because allowed returning empty
> tokens (mimiced strsep()), which is fine if there is a NULL semantic.
> Unfortunately SystemTap doesn't provide it in scripts and has only blank
> string (""), therefore testing against it was misleading.
> The solution is to return only non-empty tokens (mimic strtok()).
>
> * tapset/string.stp: Fix tokenize.
> * testsuite/systemtap.string/tokenize.stp: Improve and add case with
> more than one delimiter in the delim string.
> * stapfuncs.3stap.in: Update tokenize description.
> * doc/langref.tex: Ditto.
[...]
> - token = strsep(&str_start, THIS->delim);
> + while ((token = strsep(&str_start, THIS->delim)) && !token[0])
> + ;
do-while would be cleaner, please.
> if (token)
> - strncpy (THIS->__retvalue, token, MAXSTRINGLEN);
> + strncpy(THIS->__retvalue, token, (str_start ? str_start : str_end + 1) - token);
> %}
Why do you need the explicit length computation? There will always be a
NUL there anyway, right? I tried reverting this part, and the tests
still pass, so I don't see what this is for.
Also, while we're at it, strlcpy would be preferred for better
termination semantics (guaranteed NUL and no extra NUL padding).
Josh