[PATCH] pass constants as first argument to st_mult()

The result of st_mult() is the same no matter the order of its
arguments.  It invokes the macro unsigned_mult_overflows(), which
divides the second parameter by the first one.  Pass constants
first to allow that division to be done already at compile time.

Signed-off-by: Rene Scharfe <redacted>
---
 diffcore-rename.c | 2 +-
 refs.c            | 2 +-
 shallow.c         | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/diffcore-rename.c b/diffcore-rename.c
index 58ac0a5..73d003a 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c

@@ -541,7 +541,7 @@ void diffcore_rename(struct diff_options *options)
 				rename_dst_nr * rename_src_nr, 50, 1);
 	}
 
-	mx = xcalloc(st_mult(num_create, NUM_CANDIDATE_PER_DST), sizeof(*mx));
+	mx = xcalloc(st_mult(NUM_CANDIDATE_PER_DST, num_create), sizeof(*mx));
 	for (dst_cnt = i = 0; i < rename_dst_nr; i++) {
 		struct diff_filespec *two = rename_dst[i].two;
 		struct diff_score *m;

diff --git a/refs.c b/refs.c
index 814cad3..b4e7cac 100644
--- a/refs.c
+++ b/refs.c

@@ -922,7 +922,7 @@ char *shorten_unambiguous_ref(const char *refname, int strict)
 			/* -2 for strlen("%.*s") - strlen("%s"); +1 for NUL */
 			total_len += strlen(ref_rev_parse_rules[nr_rules]) - 2 + 1;
 
-		scanf_fmts = xmalloc(st_add(st_mult(nr_rules, sizeof(char *)), total_len));
+		scanf_fmts = xmalloc(st_add(st_mult(sizeof(char *), nr_rules), total_len));
 
 		offset = 0;
 		for (i = 0; i < nr_rules; i++) {

diff --git a/shallow.c b/shallow.c
index 4d554ca..54e2db7 100644
--- a/shallow.c
+++ b/shallow.c

@@ -389,7 +389,7 @@ static void paint_down(struct paint_info *info, const unsigned char *sha1,
 	unsigned int i, nr;
 	struct commit_list *head = NULL;
 	int bitmap_nr = (info->nr_bits + 31) / 32;
-	size_t bitmap_size = st_mult(bitmap_nr, sizeof(uint32_t));
+	size_t bitmap_size = st_mult(sizeof(uint32_t), bitmap_nr);
 	uint32_t *tmp = xmalloc(bitmap_size); /* to be freed before return */
 	uint32_t *bitmap = paint_alloc(info);
 	struct commit *c = lookup_commit_reference_gently(sha1, 1);

-- 
2.9.2

On Sat, Jul 30, 2016 at 08:18:31PM +0200, René Scharfe wrote:

The result of st_mult() is the same no matter the order of its
arguments.  It invokes the macro unsigned_mult_overflows(), which
divides the second parameter by the first one.  Pass constants
first to allow that division to be done already at compile time.

I'm not opposed to this, as it's easy to do (though I suspect new calls
may be introduced that violate it).

But if we really are worried about the performance of st_mult(), I
think:

  static inline size_t st_mult(size_t a, size_t b)
  {
	size_t result;
	if (!__builtin_mul_overflow(a, b, &result))
		die("whoops!");
	return result;
  }

is the right direction. I just haven't gotten around to producing a
polished patch.

I didn't look at all of the calls, but I wonder if it is a natural
pattern to put the constant second.  Since multiplication is
commutative, it would be correct for st_mult() to just flip the order of
arguments it feeds to unsigned_mult_overflows().

That may introduce the same inefficiency in other callsites, but I
wonder if it would be fewer.

-Peff