On Wed, Feb 8, 2017 at 5:04 AM, René Scharfe <l.s.r@xxxxxx> wrote:
> Pass the match member of the first pathspec item directly to
> read_directory() instead of using common_prefix() to duplicate it first,
> thus avoiding memory duplication, strlen(3) and free(3).
How about killing common_prefix()? There are two other callers in
ls-files.c and commit.c and it looks safe to do (but I didn't look
very hard).
> diff --git a/dir.c b/dir.c
> index 65c3e681b8..4541f9e146 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -174,20 +174,19 @@ char *common_prefix(const struct pathspec *pathspec)
>
> int fill_directory(struct dir_struct *dir, const struct pathspec *pathspec)
> {
> - char *prefix;
> + const char *prefix;
> size_t prefix_len;
>
> /*
> * Calculate common prefix for the pathspec, and
> * use that to optimize the directory walk
> */
> - prefix = common_prefix(pathspec);
> - prefix_len = prefix ? strlen(prefix) : 0;
> + prefix_len = common_prefix_len(pathspec);
> + prefix = prefix_len ? pathspec->items[0].match : "";
There's a subtle difference. Before the patch, prefix[prefix_len] is
NUL. After the patch, it's not always true. If some code (incorrectly)
depends on that, this patch exposes it. I had a look inside
read_directory() though and it looks like no such code exists. So, all
good.
>
> /* Read the directory and prune it */
> read_directory(dir, prefix, prefix_len, pathspec);
>
> - free(prefix);
> return prefix_len;
> }
--
Duy