The following review has been posted through the commitfest application:
make installcheck-world:  not tested
Implements feature:       not tested
Spec compliant:           not tested
Documentation:            not tested

This applies cleanly to master, builds and passes regression tests in my 
Windows/Cygwin environment.  

I also read through comments and confirmed for myself that the assumption about 
the caller ensuring var1 is shorter is done already in unchanged code from 
mul_var.  Frees correspond to inits.  The "Karatsuba condition" reasoning for 
deciding whether a number is big enough to use this algorithm appears to match 
what Joel has stated in this thread.

The arithmetic appears to match what's been described in the comments.  I have 
*not* confirmed that with any detailed review of the Karatsuba algorithm from 
outside sources, other implementations like the Rust one referenced here, or 
anything similar.  I'm hoping that the regression tests give sufficient 
coverage that if the arithmetic was incorrect there would be obvious failures.  
If additional coverage was needed, cases falling immediately on either side of 
the limiting conditions used in the patch would probably be useful.  From the 
limited precedent I've exposed myself to, that doesn't seem to be required 
here, but I'm open to contrary input from other reviewers.  In the meantime, 
I'm marking this approved.  

Thanks for the detailed background and comments, Joel!

The new status of this patch is: Ready for Committer

Reply via email to