Re: [PATCH 3/5] sparse-index: use strbuf in path_found()

>
> From: Derrick Stolee <stolee@gmail.com>
>
> The path_found() method previously reused strings from the cache entries
> the calling methods were using. This prevents string manipulation in
> place and causes some odd reallocation before the final lstat() call in
> the method.
>
> Refactor the method to use strbufs and copy the path into the strbuf,
> but also only the parent directory and not the whole path. This looks
> like extra copying when assigning the path to the strbuf, but we save an
> allocation by dropping the 'tmp' string, and we are "reusing" the copy
> from 'tmp' to put the data in the strbuf.
>
> Signed-off-by: Derrick Stolee <stolee@gmail.com>
> ---
>  sparse-index.c | 21 +++++++++------------
>  1 file changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/sparse-index.c b/sparse-index.c
> index de6e727f5c1..fec4f393360 100644
> --- a/sparse-index.c
> +++ b/sparse-index.c
> @@ -440,31 +440,30 @@ void ensure_correct_sparsity(struct index_state *istate)
>  }
>
>  struct path_found_data {
> -       const char *dirname;
> -       size_t dir_len;
> +       struct strbuf dir;
>         int dir_found;
>  };
>
>  #define PATH_FOUND_DATA_INIT { \
> +       .dir = STRBUF_INIT, \
>         .dir_found = 1 \
>  }
>
>  static void clear_path_found_data(struct path_found_data *data)
>  {
> -       return;
> +       strbuf_release(&data->dir);
>  }
>
>  static int path_found(const char *path, struct path_found_data *data)
>  {
>         struct stat st;
>         char *newdir;
> -       char *tmp;
>
>         /*
>          * If dirname corresponds to a directory that doesn't exist, and this
>          * path starts with dirname, then path can't exist.
>          */
> -       if (!data->dir_found && !memcmp(path, data->dirname, data->dir_len))
> +       if (!data->dir_found && !memcmp(path, data->dir.buf, data->dir.len))
>                 return 0;
>
>         /*
> @@ -486,17 +485,15 @@ static int path_found(const char *path, struct path_found_data *data)
>          * If path starts with directory (which we already lstat'ed and found),
>          * then no need to lstat parent directory again.
>          */
> -       if (data->dir_found && data->dirname &&
> -           memcmp(path, data->dirname, data->dir_len))
> +       if (data->dir_found && data->dir.buf &&
> +           memcmp(path, data->dir.buf, data->dir.len))
>                 return 0;
>
>         /* Free previous dirname, and cache path's dirname */
> -       data->dirname = path;
> -       data->dir_len = newdir - path + 1;
> +       strbuf_reset(&data->dir);
> +       strbuf_add(&data->dir, path, newdir - path + 1);
>
> -       tmp = xstrndup(path, data->dir_len);
> -       data->dir_found = !lstat(tmp, &st);
> -       free(tmp);
> +       data->dir_found = !lstat(data->dir.buf, &st);
>
>         return 0;
>  }
> --
> gitgitgadget