On Tue, Apr 1, 2014 at 2:13 PM, Gehad <gehadelro...@gmail.com> wrote:

>
> On 04/01/2014 06:50 AM, Dirk Hohndel wrote:
>
>> b) I know that different people have different preferences there, but
>> from an API perspective I would prefer replace_char(string, old, new)
>>
> I changed the order of the arguments, I am not sure if you me to change
> the names too.
>
>  c) I'm not in love with the realloc inside the loop. I realize that the
>> code is correct and that this maybe isn't the most performance critical
>> part of our application, but still... you could either brute force it
>> (worst case assumption of len(string)/len(old)*len(new) or you could
>> count the occurrences of old and then calculate the correct size of the
>> allocation
>>
> I don't usually use realloc() but I found it used a lot in the membuffer
> functions, I changed it to one memory allocation in the start after
> calculating the new size.
> I tried to follow the coding style in this patch.


You could optimize the copy loop by copying data always to the result.
Something like follows:

strcpy(temp, str);
while (ptr && ptr_old = ptr) {
    *ptr = '\0';
    strcat(result, ptr_old);
    strcat(result, replace_by);
    ptr = strchr(temp, replace);
}


 The return clause in the following seems to be 4 spaces, instead of proper
tab.

+       free(temp);
+    return result;

miika
_______________________________________________
subsurface mailing list
subsurface@hohndel.org
http://lists.hohndel.org/cgi-bin/mailman/listinfo/subsurface

Reply via email to