[pacman-dev] [PATCH] rewrite strreplace

[Dan McGee]

On Sun, Sep 6, 2009 at 6:47 PM, Xavier Chantry<shiningxc at gmail.com> wrote:
> * just do one malloc call
>
> * p = realloc(p, new_size) was not good
> (see http://www.iso-9899.info/wiki/Why_not_realloc)
>
> * use more efficient strncpy instead of strncat
>
> Signed-off-by: Xavier Chantry <shiningxc at gmail.com>
> ---
>  src/pacman/util.c |   75 +++++++++++++++++++++++++++++++----------------------
>  1 files changed, 44 insertions(+), 31 deletions(-)
>
> diff --git a/src/pacman/util.c b/src/pacman/util.c
> index 6af8229..d137460 100644
> --- a/src/pacman/util.c
> +++ b/src/pacman/util.c
> @@ -335,48 +335,61 @@ char *strtrim(char *str)
>        return(str);
>  }
>
> -/* Helper function for strreplace */
> -static void _strnadd(char **str, const char *append, unsigned int count)
> -{
> -       if(*str) {
> -               *str = realloc(*str, strlen(*str) + count + 1);
> -       } else {
> -               *str = calloc(sizeof(char), count + 1);
> -       }
> -
> -       strncat(*str, append, count);
> -}
> -
>  /* Replace all occurances of 'needle' with 'replace' in 'str', returning
>  * a new string (must be free'd) */
>  char *strreplace(const char *str, const char *needle, const char *replace)
>  {
> -       const char *p, *q;
> -       p = q = str;
> -
> -       char *newstr = NULL;
> +       const char *p = NULL, *q = NULL;
> +       char *newstr = NULL, *newp = NULL;
> +       alpm_list_t *i = NULL, *list = NULL;
>        unsigned int needlesz = strlen(needle),
>                                                         replacesz = strlen(replace);
>
> -       while (1) {
> +       if(!str) {
> +               return(NULL);
> +       }
> +
> +       p = str;
> +       q = strstr(p, needle);
> +       while(q) {
> +               list = alpm_list_add(list, (char *)q);
> +               p = q + needlesz;
>                q = strstr(p, needle);
> -               if(!q) { /* not found */
> -                       if(*p) {
> -                               /* add the rest of 'p' */
> -                               _strnadd(&newstr, p, strlen(p));
> -                       }
> -                       break;
> -               } else { /* found match */
> -                       if(q > p){
> -                               /* add chars between this occurance and last occurance, if any */
> -                               _strnadd(&newstr, p, q - p);
> -                       }
> -                       _strnadd(&newstr, replace, replacesz);
> -                       p = q + needlesz;
> +       }
> +
> +       if(!list) {
> +               return(strdup(str));
> +       }
> +       newstr = malloc(strlen(str) + 1 +
> +                       alpm_list_count(list) * (replacesz - needlesz));
Maybe an explanation comment here? And there should be no way to trick
this up by having needlesize > replacesize, or the other way around
(replacesize > needlesize), since those are declared as unsigned?

> +       if(!newstr) {
> +               return(NULL);
> +       }
> +       *newstr = 0;
I'd prefer '\0' when setting a string pointer to a null character; I
think we try to do this in the rest of the code as well.

> +
> +       p = str;
> +       newp = newstr;
> +       for(i = list; i; i = alpm_list_next(i)) {
> +               q = alpm_list_getdata(i);
> +               if(q > p){
> +                       /* add chars between this occurance and last occurance, if any */
Spelling, s/occurance/occurrence/

> +                       strncpy(newp, p, q - p);
> +                       newp += q - p;
>                }
> +               strncpy(newp, replace, replacesz);
> +               newp += replacesz;
> +               p = q + needlesz;
> +       }
> +       alpm_list_free(list);
> +
> +       if(*p) {
> +               /* add the rest of 'p' */
> +               strcpy(newp, p);
> +               newp += strlen(p);
>        }
> +       *newp = 0;
Same as above.

>
> -       return newstr;
> +       return(newstr);
>  }
>
>  /** Splits a string into a list of strings using the chosen character as
> --
> 1.6.4.2