John Keeping wrote:
> On Sun, May 19, 2013 at 12:36:12PM -0700, Jonathan Nieder wrote:

>>                                        Who is responsible for freeing
>> "path"?  Would it make sense to use a strbuf here?
>>      strbuf_setlen(&buf, traverse_path_len(info, n));
>>      make_traverse_path(&buf.buf, info, n);
> Perhaps alloc_traverse_path?  I'm not sure the strbuf buys us much for
> this case, since we only use the result for a few lines before freeing
> it.

Good idea.  Generally strbufs are nice for this kind of thing because
they make it easy to reuse buffers, but in this case there's no
opportunity for that.

> then this is "process_changed_path".

Sounds good.

>> What should happen for commits with more than 1 parent?  If they're
>> supposed to not enter this codepath because of a previous check, a
>> die("BUG: ...") could be a good way to make reading easier.
> Currently the patch ID code compares the commit with its first parent,
> so I think this should do the same.

Ok.  I guess a comment nearby would help future readers avoid the same

I don't know what it should mean to try to use --cherry without
--no-merges or --first-parent, so I guess this is harmless.

> Thanks for the review.

No problem.  Thanks for a pleasant read.

To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to
More majordomo info at

Reply via email to