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

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 1, 2016, 21:00 UTC
Message-ID: <xmqq8twgi4qp.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20160801164723.mober7em6znt56w4@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 5 quoted lines

>> -	mx = xcalloc(st_mult(num_create, NUM_CANDIDATE_PER_DST), sizeof(*mx));
>> +	mx = xcalloc(st_mult(NUM_CANDIDATE_PER_DST, num_create), sizeof(*mx));
>
> I didn't look at all of the calls, but I wonder if it is a natural
> pattern to put the constant second.

Between

    st_mult(GIT_SHA1_RAWSZ, i)
    st_mult(i, GIT_SHA1_RAWSZ)

the former is more intuitive at least to me [*1*], but calloc(3) disagrees with me.

Show 6 quoted lines

> 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.

"git grep -A3 st_mult \*.c" seems to tell me that the callsites with a constant in their first parameter are the majority (many are sizeof(something)). The three places in the patch under discussion are the only places that got them in the different order.

[Footnote]

*1* I have a slight suspicion that this is cultural, i.e. how arithmetic is taught in grade schools. When an apple costs 30 yen and I have 5 of them, I was taught to multiply 30x5 to arrive at 150, not 5x30=150, and I am guessing that is because the former matches the natural order of these two numbers (cost, quantity) in the language I was taught.