On 2015-11-30 at 19:43 'Davide Libenzi' via Akaros wrote:
> Oh, I would not have made any comment, had I not seen the silly
> optimization in place. Likely 1993 era 😀
> No optimization in place, meant "we do not care at this point", which
> I agree in this case.
> But the code was suggesting "we do care", hence a much better way to
> do it.

I'm actually fine with just using:

/* We wraparound if UINT_MAX < a * b, which is also UINT_MAX / a < b. */
static inline bool mult_will_overflow_u64(uint64_t a, uint64_t b)
{
    if (!a)
        return FALSE;
    return (uint64_t)(-1) / a < b;
}

which is already in the kernel.

I went with the existing approach, since that's what Dan brought in for
krealloc array recently, but would be happy to use either or.  

My slight preference would be to just use a helper to detect overflow
(whether it's mine, Dan's from BSD, or yours that uses u128s), but I
went with a low-churn, let's not argue about it approach.

For now, I'm going to leave this as is.  

If someone wants to investigate the best way to do this in a follow-up
commit and fix it for all uses of overflow detection, then that'd be
great. 

Ideally, we'd have a helper that can take any type and tell us if the
mult will overflow.  The alloc arrays will then both use that helper.
The helper would also provide the opportunity for arch-specific
includes.  It also gets the enum and whatnot out of the .c file.  I
think that covers everyone's points from this thread.

I put it on the big TODO list on the wiki:
https://github.com/brho/akaros/wiki#overflowing-multiplication

Barret

-- 
You received this message because you are subscribed to the Google Groups 
"Akaros" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To post to this group, send email to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to