summary refs log tree commit diff
path: root/compat
diff options
context:
space:
mode:
authorJunio C Hamano <gitster@pobox.com>2022-01-07 13:30:00 -0800
committerJunio C Hamano <gitster@pobox.com>2022-01-07 14:20:58 -0800
commit9bfa5fbae21fc55141766c3192a0c26164423e37 (patch)
tree2f0dc21e44367335dbc92634af654c028f0387f9 /compat
parente9d7761bb94f20acc98824275e317fa82436c25d (diff)
compat/qsort_s.c: avoid using potentially unaligned access
The compatibility definition for qsort_s() uses "char buffer[1024]"
on the stack to avoid making malloc() calls for small temporary
space, which essentially hand-rolls alloca().

But the elements of the array being sorted may have alignment needs
more strict than what an array of bytes may have. &buf[0] may be
word aligned, but using the address as if it stores the first
element of an array of a struct, whose first member may need to be
aligned on double-word boundary, would be a no-no.

We could use xalloca() from git-compat-util.h, or alloca() directly
on platforms with HAVE_ALLOCA_H, but let's try using unconditionally
xmalloc() before we know the performance characteristics of the
callers.

It may not make much of an argument to inspect the current callers
and say "it shouldn't matter to any of them", but anyway:

 * The one in object-name.c is used to sort potential matches to a
   given ambiguous object name prefix in the error path;

 * The one in pack-write.c is done once per a pack .idx file being
   written to create the reverse index, so (1) the cost of malloc()
   overhead is dwarfed by the cost of the packing operation, and (2)
   the number of entries being sorted is the number of objects in a
   pack;

 * The one in ref-filter.c is used by "branch --list", "tag --list",
   and "for-each-ref", only once per operation.  We sort an array of
   pointers with entries, each corresponding to a ref that is shown.

 * The one in string-list.c is used by sort_string_list(), which is
   way too generic to assume any access patterns, so it may or may
   not matter, but I do not care too much ;-)

Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diffstat (limited to 'compat')
-rw-r--r--compat/qsort_s.c14
1 files changed, 4 insertions, 10 deletions
diff --git a/compat/qsort_s.c b/compat/qsort_s.c
index 52d1f0a73d..0f7ff30f5f 100644
--- a/compat/qsort_s.c
+++ b/compat/qsort_s.c
@@ -49,21 +49,15 @@ int git_qsort_s(void *b, size_t n, size_t s,
 		int (*cmp)(const void *, const void *, void *), void *ctx)
 {
 	const size_t size = st_mult(n, s);
-	char buf[1024];
+	char *tmp;
 
 	if (!n)
 		return 0;
 	if (!b || !cmp)
 		return -1;
 
-	if (size < sizeof(buf)) {
-		/* The temporary array fits on the small on-stack buffer. */
-		msort_with_tmp(b, n, s, cmp, buf, ctx);
-	} else {
-		/* It's somewhat large, so malloc it.  */
-		char *tmp = xmalloc(size);
-		msort_with_tmp(b, n, s, cmp, tmp, ctx);
-		free(tmp);
-	}
+	tmp = xmalloc(size);
+	msort_with_tmp(b, n, s, cmp, tmp, ctx);
+	free(tmp);
 	return 0;
 }