From: Junio C Hamano <gitster@pobox.com>
Date: Mon, 11 Jul 2011 19:06:08 GMT
Subject: Re: [PATCH 1/5] decorate: allow storing values instead of pointers
Message-ID: <7vr55wd1hw.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20110711161649.GA10418@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

>  static void grow_decoration(struct decoration *n)
>  {
> -	int i;
> -	int old_size = n->size;
> -	struct object_decoration *old_hash = n->hash;
> +	unsigned char *old_hash = n->hash;
> +	unsigned char *old_end = n->end;
> +	unsigned char *p;
>  
> -	n->size = (old_size + 1000) * 3 / 2;
> -	n->hash = xcalloc(n->size, sizeof(struct object_decoration));
> +	n->stride = sizeof(struct object_decoration) + WIDTH(n);

This value should not change once it is initialized, or all h*ll breaks
loose while accessing the old-hash, right?  Just wondering if it makes the
intention clearer if the function had something like this in it:

	if (!old_size) {
		/* initial */
                n->stride = ...
	} else {
		/* rehash to grow */
	}

I am mostly worried about both width and stride being assignable
fields. An alternative may be to expose

	int decoration_stride(struct decoration *n)
        {
        	return sizeof(struct object_decoration) + WIDTH(n);
	}

to the outside callers and drop "stride" field.