On Thu, Jun 20, 2024 at 10:11 AM Derrick Stolee via GitGitGadget
<gitgitgadget@xxxxxxxxx> wrote:
>
> From: Derrick Stolee <stolee@xxxxxxxxx>
>
> 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@xxxxxxxxx>
> ---
> 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
Another simple translation; the series looks good so far...
