Re: [PATCH 3/5] match-trees: use hashcpy to splice trees

From: "brian m. carlson" <sandals@crustytoothpaste.net>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org, "Martin Ågren" <martin.agren@gmail.com>,
	"Junio C Hamano" <gitster@pobox.com>
Subject: Re: [PATCH 3/5] match-trees: use hashcpy to splice trees
Date: Thu, 10 Jan 2019 23:55:51 +0000	[thread overview]
Message-ID: <20190110235551.GM423984@genre.crustytoothpaste.net> (raw)
In-Reply-To: <20190110064520.GC20497@sigill.intra.peff.net>

[-- Attachment #1: Type: text/plain, Size: 1601 bytes --]

On Thu, Jan 10, 2019 at 01:45:20AM -0500, Jeff King wrote:
> On Thu, Jan 10, 2019 at 04:25:49AM +0000, brian m. carlson wrote:
> > diff --git a/match-trees.c b/match-trees.c
> > index feca48a5fd..b1fbd022d1 100644
> > --- a/match-trees.c
> > +++ b/match-trees.c
> > @@ -224,7 +224,7 @@ static int splice_tree(const struct object_id *oid1, const char *prefix,
> >  	} else {
> >  		rewrite_with = oid2;
> >  	}
> > -	oidcpy(rewrite_here, rewrite_with);
> > +	hashcpy(rewrite_here->hash, rewrite_with->hash);
> 
> Hrm. Our coccinelle patches will want to convert this back to oidcpy(),
> though I see you drop them in the final patch.
> 
> However, I wonder if it points to another mismatch. Isn't the point that
> we _don't_ actually have "struct object_id"s here? I.e., rewrite_here
> and rewrite_with should actually be "const unsigned char *" that we
> happen to know are the_hash_algo->raw_sz?

Correct.

> I think the only reason they are "struct object_id" is because that's
> what tree_entry_extract() returns. Should we be providing another
> function to allow more byte-oriented access?

The reason is we recursively call splice_tree, passing rewrite_here as
the first argument. That argument is then used for read_object_file,
which requires a struct object_id * (because it is, logically, an object
ID).

I *think* we could fix this by copying an unsigned char *rewrite_here
into a temporary struct object_id before we recurse, but it's not
obvious to me if that's safe to do.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]