From: Nguyen Thai Ngoc Duy Date: Wed, 25 Aug 2010 22:11:07 GMT Subject: Re: [PATCH 2/7] Fix ignoring of pathspecs with rev-list --objects Message-ID: In-Reply-To: <1282704795-29661-3-git-send-email-newren@gmail.com> On Wed, Aug 25, 2010 at 12:53 PM, Elijah Newren wrote: > @@ -84,7 +85,24 @@ static void process_tree(struct rev_info *revs, > >        init_tree_desc(&desc, tree->buffer, tree->size); > > -       while (tree_entry(&desc, &entry)) { > +       for (; desc.size; update_tree_entry(&desc)) { > +               entry = desc.entry; > + > +               if (!all_interesting) { > +                       char *full_path = path_name(path, name); > +                       int full_path_len = strlen(full_path); > +                       int showit = tree_entry_interesting(&desc, > full_path, full_path_len, > +                                                           &revs->diffopt); > +                       free(full_path); I wonder if we can avoid xmalloc/free so many times here. If full_path is unchanged, how about moving it outside the loop? > diff --git a/tree-diff.c b/tree-diff.c > index cd659c6..2fb670b 100644 > --- a/tree-diff.c > +++ b/tree-diff.c > @@ -91,7 +91,7 @@ static int compare_tree_entry(struct tree_desc *t1, struct > tree_desc *t2, const >  *  - zero for no >  *  - negative for "no, and no subsequent entries will be either" >  */ > -static int tree_entry_interesting(struct tree_desc *desc, const char *base, > int baselen, struct diff_options *opt) > +int tree_entry_interesting(struct tree_desc *desc, const char *base, int > baselen, struct diff_options *opt) >  { >        const char *path; >        const unsigned char *sha1; While at it, can you please also fix its comments? The comments say pathspec while what it uses is actually path prefix. Maybe something like this diff --git a/tree-diff.c b/tree-diff.c index 3c34c77..514dbca 100644 --- a/tree-diff.c +++ b/tree-diff.c @@ -83,7 +83,7 @@ static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2, const } /* - * Is a tree entry interesting given the pathspec we have? + * Is a tree entry interesting given the path prefix we have? * * Return: * - 2 for "yes, and all subsequent entries will be" This patch is good stuff. Please be informed I will steal this patch for my narrow use. -- Duy