Jeff King <p...@peff.net> writes:

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

> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to