On Sat, Feb 20, 2016 at 10:32:00PM +0100, René Scharfe wrote:
> >-#define REALLOC_ARRAY(x, alloc) (x) = xrealloc((x), (alloc) * sizeof(*(x)))
> >+#define ALLOC_ARRAY(x, alloc) (x) = xmalloc(st_mult((alloc), sizeof(*(x))))
> >+#define REALLOC_ARRAY(x, alloc) (x) = xrealloc((x), st_mult((alloc),
> >sizeof(*(x))))
>
> st_mult(x, y) calls unsigned_mult_overflows(x, y), which divides by x. This
> division can be done at compile time if x is a constant. This can be
> guaranteed for all users of the two macros above by reversing the arguments
> of st_mult(), so that sizeof comes first. Probably not a big win, but why
> not do it if it's that easy?
I doubt it's even measurable, but as you say, it's easy enough to do, so
why not.
If we really care about optimizing, I suspect that something like:
diff --git a/git-compat-util.h b/git-compat-util.h
index 5cbdd2e..1d53328 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -662,10 +662,12 @@ static inline size_t st_add(size_t a, size_t b)
static inline size_t st_mult(size_t a, size_t b)
{
- if (unsigned_mult_overflows(a, b))
+ size_t ret;
+
+ if (__builtin_mul_overflow(a, b, &ret))
die("size_t overflow: %"PRIuMAX" * %"PRIuMAX,
(uintmax_t)a, (uintmax_t)b);
- return a * b;
+ return ret;
}
static inline size_t st_sub(size_t a, size_t b)
would do a lot more. But it needs #ifdefs for compilers besides gcc and
clang.
> Or perhaps a macro like this could help here and in other places which use
> st_mult with sizeof:
>
> #define SIZEOF_MULT(x, n) st_mult(sizeof(x), (n))
>
> (I'd call it ARRAY_SIZE, but that name is already taken. :)
I don't think we need that; we're really only checking allocations,
which means ALLOC_ARRAY() and friends cover all the uses of sizeof (we
might still use st_mult() for _another_ part of the computation, but it
won't usually be a sizeof then).
We also may do follow-up multiplications, like:
ALLOC_ARRAY(foo, nr);
memset(foo, 0, nr * sizeof(*foo));
but I didn't bother doing overflow checks for those. We know that they
should be fine if the original allocation was successful (though of
course, just using xcalloc here would be better still).
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
