Re: [PATCH] make sha1_sort inline

From: David Rientjes <rientjes@google.com>
Date: Aug 18, 2006, 16:56 UTC
Message-ID: <Pine.LNX.4.63.0608180947240.29405@chino.corp.google.com>
In-Reply-To: <7vsljud2fs.fsf@assigned-by-dhcp.cox.net>

On Thu, 17 Aug 2006, Junio C Hamano wrote:

Show 8 quoted lines

> The only use of this function is to got its address taken of,
> and then the variable that has its address (current_sort) is
> used by sort_comparator() function, whose address is given to
> the library routine qsort(3).
> 
> Does it still make sense to declare it inline?
> 
>

No.

Well, maybe just "not yet." The idea is to isolate the sort functions for hashes since we may add support for additional ones in the future (re: my thread on md5). Then all these functions would probably be abstracted to a single file and labeled as static with a single global function:

	static int hash_sort(const struct object_entry,
			     const struct object_entry);

which would call the static inline function that corresponds to the hash in use for that database. This can't simply be #ifdef's since the choice of hash would not be compile-time, it would be run-time depending on what flags are sent during init-db.

And instead of:
	struct object_entry {
		unsigned char sha1[20];
		...
	}

it becomes:
	struct object_entry {
		union hash_name {
			unsigned char sha1[20];
			...
		}
		...
	}

So in that case, the static inline of the sort does have its advantage as opposed to two function calls on any hash_sort. But I agree in the meantime while additional hashes are still be considered that this would not be an advantageous addition to the code.

(At least you get an idea of what I'm thinking about hashes)

		David