Re: [PATCH] dir: avoid allocation in fill_directory()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]