Re: [PATCH] Properly align memory allocations and temporary buffers
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 7, 2022, 19:30 UTC
- Message-ID
- <xmqq7dbb9wcz.fsf@gitster.g>
- In-Reply-To
- <Ydea3EO7wrYc9/Ij@camp.crustytoothpaste.net>
"brian m. carlson" <sandals@crustytoothpaste.net> writes:
> I'm not opposed to a small amount of finagling for this case, but I am > very much opposed to defining your C ABI in an intentionally difficult > way.
I was looking at git_qsort_s() today and thought that its use of "char buf[1k]" on stack is something we would write for a system without alloca().
We already have xalloca() wrapper defined in the compat-util header. Perhaps it is a good idea to use it instead?
Both the true alloca() and xmalloc() we use as a fallback (when <alloca.h> is not available) are supposed to return a block of memory that is suitably aligned for any use, no?
compat/qsort_s.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)
diff --git c/compat/qsort_s.c w/compat/qsort_s.c index 52d1f0a73d..63660a4304 100644 --- c/compat/qsort_s.c +++ w/compat/qsort_s.c @@ -49,21 +49,23 @@ 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; + int use_alloca; 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); + use_alloca = (size < 1024); + + tmp = use_alloca ? xalloca(size) : xmalloc(size); + + msort_with_tmp(b, n, s, cmp, tmp, ctx); + + if (use_alloca) + xalloca_free(tmp); + else free(tmp); - } return 0; }