This is the mail archive of the
systemtap@sourceware.org
mailing list for the systemtap project.
Re: [PATCH] Add Strrplc() To Tapsets
On Wednesday 17 Jun 2009 7:48:23 am Josh Stone wrote:
> 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.
>
Ah, something i didnt notice. Thanks for that. Now, this makes me wonder,
THIS->__retvalue max size would be MAXSTRINGLEN right? If so, if we have to
replace a sting which would result in a string much bigger than MAXSTRINGLEN
would cause an overflow right? Should we check that case too? Or am i missing
something?
> > 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);
I didnt want to modify the original parameters, so ideally i would have copied
the full string into some array and then manipulated as you specified above.
I chose temp to avoid that copy. Since you suggest that original parameter can
be changed then we can remove temp. I will make the change and submit.
>
> > 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?
>
Will be done.
> strlcat(THIS->__retvalue, ptr_base, MAXSTRINGLEN);
>
>
> Josh
--
Regards,
Varun Chandramohan