On Thu, Feb 13, 2014 at 09:43:27AM -0800, Junio C Hamano wrote: > Kirill Smelkov <kirr@xxxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html