Re: [PATCH 4/5] tree-walk: store object_id in a separate member
- From
Jeff King <peff@peff.net>
- Date
- Jan 10, 2019, 06:49 UTC
- Message-ID
- <20190110064944.GA6810@sigill.intra.peff.net>
- In-Reply-To
- <20190110042551.915769-5-sandals@crustytoothpaste.net>
On Thu, Jan 10, 2019 at 04:25:50AM +0000, brian m. carlson wrote:
Show 14 quoted lines
> diff --git a/tree-walk.c b/tree-walk.c > index 1e040fc20e..b6daeab16d 100644 > --- a/tree-walk.c > +++ b/tree-walk.c > @@ -48,7 +48,8 @@ static int decode_tree_entry(struct tree_desc *desc, const char *buf, unsigned l > /* Initialize the descriptor entry */ > desc->entry.path = path; > desc->entry.mode = canon_mode(mode); > - desc->entry.oid = (const struct object_id *)(path + len); > + desc->entry.pathlen = len - 1; > + memcpy(desc->entry.oid.hash, path + len, the_hash_algo->rawsz); > > return 0; > }
This one really is a hashcpy() now, right, even after your final patch? I guess using rawsz explicitly makes it match the computation here:
Show 8 quoted lines
> @@ -107,7 +108,7 @@ static void entry_extract(struct tree_desc *t, struct name_entry *a)
> static int update_tree_entry_internal(struct tree_desc *desc, struct strbuf *err)
> {
> const void *buf = desc->buffer;
> - const unsigned char *end = desc->entry.oid->hash + the_hash_algo->rawsz;
> + const unsigned char *end = (const unsigned char *)desc->entry.path + desc->entry.pathlen + 1 + the_hash_algo->rawsz;
> unsigned long size = desc->size;
> unsigned long len = end - (const unsigned char *)buf;So maybe it's better to be explicit as you have here. (Mostly just as I was reading it, I was looking for a use of hashcpy and was surprised not to find it ;) ).
Show 13 quoted lines
> @@ -175,9 +176,11 @@ void setup_traverse_info(struct traverse_info *info, const char *base)
> pathlen--;
> info->pathlen = pathlen ? pathlen + 1 : 0;
> info->name.path = base;
> - info->name.oid = (void *)(base + pathlen + 1);
> - if (pathlen)
> + info->name.pathlen = pathlen;
> + if (pathlen) {
> + memcpy(info->name.oid.hash, (base + pathlen + 1), the_hash_algo->rawsz);
> info->prev = &dummy;
> + }
> }
> Ditto here, I think.
-Peff