From: brian m. carlson Date: Wed, 22 Feb 2017 23:22:15 GMT Subject: Re: [PATCH v5 03/19] builtin/diff-tree: convert to struct object_id Message-ID: <20170222232215.u2agozagwsyy2ooe@genre.crustytoothpaste.net> In-Reply-To: <20170222191641.o2rtt2ymtb4h2yqe@sigill.intra.peff.net> On Wed, Feb 22, 2017 at 02:16:42PM -0500, Jeff King wrote: > On Wed, Feb 22, 2017 at 10:50:21AM -0800, Junio C Hamano wrote: > > > "brian m. carlson" 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). I did take that part, as well as other parts, from your patch. I have a test, which I'll send in a minute, that verifies that they do the same thing. At first I introduced a segfault, and the test caught it, so I feel confident that the patch does indeed function as the old code did. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: https://keybase.io/bk2204