This is the mail archive of the
systemtap@sourceware.org
mailing list for the systemtap project.
Re: [PATCH] Add Strrplc() To Tapsets
On 06/16/2009 03:47 AM, Varun Chandramohan wrote:
> Hi Josh,
>
> Here is a modified version of the function. I made the changes you
> suggested me. I have not made a patch as i wanted to check with you first.
> Let me know if its ok? I will submit proper patch along with testcase later.
Thanks. When you write your test cases, please try to include corner
cases like truncation and a 0-length search string.
> function str_replace:string (prnt_str:string, srch_str:string,rplc_str:string)
> %{
> char temp[MAXSTRINGLEN];
If you must use a temp array, then it will have to be static, since
MAXSTRINGLEN could be too large to fit on the kernel stack. But I still
don't think you need this array.
> while((ptr = strstr(ptr, THIS->srch_str)) != NULL) {
This will loop forever if strlen(THIS->srch_str)==0. Since you also
need that length repeatedly to advance ptr, I would save and check that
length ahead of time. If 0, set CONTEXT->last_error and return.
> memset(temp, 0, MAXSTRINGLEN);
> strlcpy(temp, ptr_base, ptr - ptr_base + 1);
> strlcat(THIS->__retvalue, temp, MAXSTRINGLEN);
It's ok to modify the parameters, so how about this:
*ptr = '\0';
strlcat(THIS->__retvalue, ptr_base, MAXSTRINGLEN);
> if(ptr_base == THIS->prnt_str) {
> strlcpy(THIS->__retvalue, THIS->prnt_str, MAXSTRINGLEN);
> } else {
> ptr = THIS->prnt_str;
> ptr = ptr + strlen(THIS->prnt_str);
> strlcpy(temp, ptr_base, ptr - ptr_base + 1);
> strlcat(THIS->__retvalue, temp, MAXSTRINGLEN);
> }
This seems strangely complicates -- why not just this?
strlcat(THIS->__retvalue, ptr_base, MAXSTRINGLEN);
Josh