Re: [PATCH 1/2] Add `struct object_hash`

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Feb 18, 2007, 16:48 UTC
Message-ID: <Pine.LNX.4.63.0702181740100.22628@wbgn013.biozentrum.uni-wuerzburg.de>
In-Reply-To: <7vtzxjj24j.fsf@assigned-by-dhcp.cox.net>

Hi,

These are only minor nits, as I think your code is bug free.

On Sun, 18 Feb 2007, Junio C Hamano wrote:

> +static void insert_object_into_hash(struct object_hash_entry *ent, struct object_hash *hash)

I'd rather _not_ name it "ent", as that has a different meaning already.

Also, what is your preferred width? Your line is way longer than 80 columns.

Show 9 quoted lines

> +static void grow_object_hash(struct object_hash *hash)
> +{
> +	struct object_hash_entry **old_hash = hash->hash;
> +	int i;
> +	int old_size = hash->size;
> +	int new_size = (old_size + 1000) * 3 / 2;
> +
> +	hash->hash = xcalloc(new_size, sizeof(struct object_hash_entry *));
> +	hash->size = new_size;

Either have a "new_hash" variable, or get rid of "new_size".

BTW I just tried to convert obj_hash to this interface, and it seems that there really is no elegant way (without wasting memory; You could do it, but then you'd have _two_ pointers for each object: one to the object_hash_entry, and from _that_ to the object). Maybe you have a clever way to solve this?

Ciao, Dscho