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

[Brandon Williams <bmwill@google.com>]

On 12/07, Duy Nguyen wrote:
> On Wed, Dec 7, 2016 at 4:51 AM, Brandon Williams <bmwill@google.com> wrote:
> > 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.

I didn't even notice that.  Now looking at this you're right that its
not immediately obvious that what's there is correct.  I'll change this.

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

-- 
Brandon Williams