This is the mail archive of the systemtap@sourceware.org mailing list for the systemtap project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]