Re: [PATCH 03/17] dir: convert fill_directory to use the pathspec struct interface

From: Duy Nguyen <pclouds@gmail.com>
Date: Dec 7, 2016, 10:24 UTC
Message-ID: <CACsJy8BsGVrUoFFFEqdLS-h4XYCkFg-gbx+BeWmGd8srupNWqw@mail.gmail.com>
In-Reply-To: <1481061106-117775-4-git-send-email-bmwill@google.com>

On Wed, Dec 7, 2016 at 4:51 AM, Brandon Williams <bmwill@google.com> wrote:

Show 19 quoted lines

> Convert 'fill_directory()' to use the pathspec struct interface from
> using the '_raw' entry in the pathspec struct.
>
> Signed-off-by: Brandon Williams <bmwill@google.com>
> ---
>  dir.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/dir.c b/dir.c
> index 7df292b..8730a4f 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -188,7 +188,8 @@ int fill_directory(struct dir_struct *dir, const struct pathspec *pathspec)
>         len = common_prefix_len(pathspec);
>
>         /* Read the directory and prune it */
> -       read_directory(dir, pathspec->nr ? pathspec->_raw[0] : "", len, pathspec);
> +       read_directory(dir, pathspec->nr ? pathspec->items[0].match : "",
> +                      len, pathspec);

Or even better, use common_prefix()'s return value here. I took me a while to realize this code was not buggy. It is fine to just pick the first item because the first <len> characters of _all_ pathspec items must be the same. Something like this

prefix = common_prefix(..) read_directory(..., prefix, strlen(prefix), pathspec);

expresses it much better. Yeah one extra mem allocation, no big deal since fill_directory() is not called very often.

Show 6 quoted lines

>         return len;
>  }
>
> --
> 2.8.0.rc3.226.g39d4020
>

-- 
Duy