Dean Rasheed <dean.a.rasheed@gmail.com> writes:
> On 17 November 2015 at 14:43, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Hm, good point. I don't feel a compulsion to have a test case that
>> proves it's broken before we fix it. Do you want to send a patch?
> OK, here it is. It's slightly different from mul_var, because maxdiv
> is tracking absolute values and the carry may be positive or negative,
> and it's absolute value may be as high as INT_MAX / NBASE + 1 (when
> it's negative), but otherwise the principle is the same.
I pushed this, but while looking at it, my attention was drawn to this
bit down near the end of the loop:
/* * The dividend digit we are about to replace might still be nonzero. * Fold it into the next
digitposition. We don't need to worry about * overflow here since this should nearly cancel with the
subtraction * of the divisor. */ div[qi + 1] += div[qi] * NBASE;
In the first place, I'm not feeling especially convinced by that handwave
about there being no risk of overflow. In the second place, this is
indisputably failing to consider whether maxdiv might need to increase.
If I add
Assert(Abs(div[qi + 1]) <= (maxdiv * (NBASE-1)));
right after this, the regression tests blow up. So it looks to me like
we've got some more work to do.
regards, tom lane
