Re: [PATCH v5 03/19] builtin/diff-tree: convert to struct object_id

From: brian m. carlson <sandals@crustytoothpaste.net>
Date: Feb 22, 2017, 23:22 UTC
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:

Show 20 quoted lines

> On Wed, Feb 22, 2017 at 10:50:21AM -0800, Junio C Hamano wrote:
> 
> > "brian m. carlson" <sandals@crustytoothpaste.net> 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