On Thu, Feb 13, 2014 at 09:43:27AM -0800, Junio C Hamano wrote:
> Kirill Smelkov <k...@mns.spb.ru> writes:
> 
> > +static void show_path(struct strbuf *base, struct diff_options *opt,
> > +                 struct tree_desc *t1, struct tree_desc *t2)
> >  {
> >     unsigned mode;
> >     const char *path;
> > -   const unsigned char *sha1 = tree_entry_extract(desc, &path, &mode);
> > -   int pathlen = tree_entry_len(&desc->entry);
> > +   const unsigned char *sha1;
> > +   int pathlen;
> >     int old_baselen = base->len;
> > +   int isdir, recurse = 0, emitthis = 1;
> > +
> > +   /* at least something has to be valid */
> > +   assert(t1 || t2);
> > +
> > +   if (t2) {
> > +           /* path present in resulting tree */
> > +           sha1 = tree_entry_extract(t2, &path, &mode);
> > +           pathlen = tree_entry_len(&t2->entry);
> > +           isdir = S_ISDIR(mode);
> > +   }
> > +   else {
> > +           /* a path was removed - take path from parent. Also take
> > +            * mode from parent, to decide on recursion.
> > +            */
> > +           tree_entry_extract(t1, &path, &mode);
> > +           pathlen = tree_entry_len(&t1->entry);
> > +
> > +           isdir = S_ISDIR(mode);
> > +           sha1 = NULL;
> > +           mode = 0;
> > +   }
> > +
> > +   if (DIFF_OPT_TST(opt, RECURSIVE) && isdir) {
> > +           recurse = 1;
> > +           emitthis = DIFF_OPT_TST(opt, TREE_IN_RECURSIVE);
> > +   }
> >  
> >     strbuf_add(base, path, pathlen);
> > -   if (DIFF_OPT_TST(opt, RECURSIVE) && S_ISDIR(mode)) {
> > -           if (DIFF_OPT_TST(opt, TREE_IN_RECURSIVE))
> > -                   opt->add_remove(opt, *prefix, mode, sha1, 1, base->buf, 
> > 0);
> >  
> > +   if (emitthis)
> > +           emit_diff(opt, base, t1, t2);
> > +
> > +   if (recurse) {
> >             strbuf_addch(base, '/');
> > -           diff_tree_sha1(*prefix == '-' ? sha1 : NULL,
> > -                          *prefix == '+' ? sha1 : NULL, base->buf, opt);
> > -   } else
> > -           opt->add_remove(opt, prefix[0], mode, sha1, 1, base->buf, 0);
> > +           diff_tree_sha1(t1 ? t1->entry.sha1 : NULL,
> > +                          t2 ? t2->entry.sha1 : NULL, base->buf, opt);
> > +   }
> 
> 
> After this step, "sha1" is assigned but never gets used.  Please
> double-check the fix-up I queued in the series before merging it to
> 'pu'.

Your fixup is correct - it was my overlook when preparing the patch -
sha1 is needed for later patch (multiparent diff tree-walker), but I've
mistakenly left it here.

The two interesting patches I've sent you today, are already adjusted to
this correction - it is safe to squash the fixup in.

Thanks for noticing,
Kirill
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to