[PATCH] make sha1_sort inline

David Rientjes· Aug 18, 2006, 05:30 UTC · lore

Inlines sha1_sort as discussed in previous thread.

Signed-off-by: David Rientjes <rientjes@google.com>
---
 builtin-pack-objects.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

Show changes to builtin-pack-objects.c +2 −1

diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index f19f0d6..b7ba558 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -929,7 +929,8 @@ static struct object_entry **create_sort
 	return list;
 }
 
-static int sha1_sort(const struct object_entry *a, const struct object_entry *b)
+static inline int sha1_sort(const struct object_entry *a,
+			    const struct object_entry *b)
 {
 	return hashcmp(a->sha1, b->sha1);
 }

-- 
1.4.2.rc4.gd070-dirty

Junio C Hamano· Aug 18, 2006, 05:43 UTC · re: David Rientjes · lore

Re: [PATCH] make sha1_sort inline

David Rientjes <rientjes@google.com> writes:

> Inlines sha1_sort as discussed in previous thread.

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?

David Rientjes· Aug 18, 2006, 16:56 UTC · re: Junio C Hamano · lore

Re: [PATCH] make sha1_sort inline

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

patchmake sha1_sort inline