On 31/12/2014 02:15, joe darcy wrote:
Hi Brian,

The new changes generally look good. A few comments, for the new code like

 291         } else if ((off < 0) || (off > val.length) || (len < 0) ||
292 ((off + len) > val.length) || ((off + len) < 0)) {
 293             throw new IndexOutOfBoundsException();

it is not immediately clear that the arithmetic on line 292 won't have some inappropriate overflow behavior. A comment stating why "off + len" will behave appropriately (assuming it does behave appropriately ;-) would help. (By line 292, both off and len are non-negative so that should limit the case analysis.)

I assume this can be reduced down to:
  if (off < 0 || len < 0 || (off > val.length - len)) { ... }

-Alan


Reply via email to