On Wed, Feb 22, 2017 at 10:50:21AM -0800, Junio C Hamano wrote: > "brian m. carlson" <sandals@xxxxxxxxxxxxxxxxxxxx> writes: > > > Convert most leaf functions to struct object_id. Change several > > hardcoded numbers to uses of parse_oid_hex. In doing so, verify that we > > when we want two trees, we have exactly two trees. > > > > Finally, in stdin_diff_commit, avoid accessing the byte after the NUL. > > This will be a NUL as well, since the first NUL was a newline we > > overwrote. However, with parse_oid_hex, we no longer need to increment > > the pointer directly, and can simply increment it as part of our check > > for the space character. > > After reading the pre- and post-image twice, I think I convinced > myself that this is a faithful conersion and they do the same thing. I think this is correct, too (but then, I think it largely comes from the patch I wrote the other night. So I did look at it carefully, but it's not exactly an independent review). > What the function does appears somewhat iffy in the modern world. > We are relying on the fact that while Git is operating in this mode > of reading a tuple of commits per line and doing log-tree, that Git > itself will not do the history traversal (instead, another instance > of Git that feeds us via our standard input is walking the history) > for this piece of code to work correctly. > > Of course, the "diff-tree --stdin" command was meant to sit on the > downstream of "rev-list --parents", so the assumption the code makes > (i.e. the parents field of the in-core commit objects do not have to > be usable for history traversal) may be reasonable, but still... I'm not sure it's that weird. "diff-tree" is about diffing, not traversal. The only reason it touches commit->parents at all (and doesn't just kick off a diff between the arguments it gets) is that it's been stuck with pretty-printing the commits, which might ask to show the parents. -Peff