Re: [PATCH] crypto : ecc - Fix carry overflow in vli multiplication

[Lukas Wunner]

On Tue, May 12, 2026 at 06:20:14PM +0300, Anastasia wrote:
> However, I have a few questions regarding the proposed
> check_add_128_128_overflow():
> 
> Should this function return u64 (carry flag) instead of bool to be
> consistent with existing overflow-checking functions like vli_add()?

I think if the return value can only be 1 or 0 (carry or no carry),
then bool is clearer.  If the carry can be > 1 then u64 would be
merited.

I think it's confusing that vli_add() returns u64, but this was just
copy-pasted from the micro-ecc library, whose uECC_vli_add() returns
uECC_word_t:

https://github.com/kmackay/micro-ecc/blob/master/uECC.c#L333

> Regarding argument order: if the function returns a result, shouldn't it be
> the first argument rather than the third (like vli_add())?

I think by convention, the result or destination is the first argument,
as e.g. in memcpy().  I don't know why check_add_overflow() doesn't
adhere to that convention but suspect there's probably no good reason.

> And replace:
> r01 = add_128_128(r01, product);
> r2 += (r01.m_high < product.m_high);
> with:
> r2 += check_add_128_128_overflow(&r01, r01, product);
> in functions vli_mult, vli_umult and vli_square

LGTM.

BTW a small nit, the commit subject contains a superfluous blank
in-between "crypto" and the succeeding colon.

Thanks,

Lukas