Mats Kindahl <mats@timescale.com> writes: > However, better safe than sorry, so I modified the patch to include the > check. And yes, you're right in that there is no need to check for the > operand diff since the previous checks guarantee that the operand is > between the bounds, and since the diff between the bounds is not infinite, > the diff between the operand and any of the bounds cannot be infinite. > Added a comment to that effect to the patch as well.
I looked this over and noted two problems:
* You missed fixing the mirror code path (bound1 > bound2).
* It seems at least possible that, for an operand just slightly less than bound2, the quotient ((operand - bound1) / (bound2 - bound1)) could round to exactly 1, even though it should theoretically always be in [0, 1). If that did happen, and count is INT_MAX, then the final addition of 1 would create its own possibility of integer overflow. We have code to check that but it's only applied in the operand >= bound2 case. I fixed that by moving the overflow-aware addition of 1 to the bottom of the function so it's done in all cases, and adjusting the other code paths to account for that.
Pushed with those changes and some cosmetic tweaking.
