Looks good.
On Fri, May 24, 2013 at 11:53 AM, Xueming Shen <[email protected]>wrote: > Hi, > > Please help review the change for > > JDK-8001750: CharsetDecoder.replacement should not be changeable except > via replaceWith method > > http://cr.openjdk.java.net/~**sherman/8001750/webrev/<http://cr.openjdk.java.net/~sherman/8001750/webrev/> > > The bug description is actually not accurate, the replacement field > of CharsetDecoder class is String, not a byte[], so CharsetDecoder > does not have the "defensive copy" issue described. CharsetEncoder > class however appears to have this issue (replacementWith() and > replacement() don't make defensive copy when set/get its internal > data field), which has the byte[] type replacement. > > The change is to > (1) return a copy of the replacement when replacement() is invoked > (2) save a defensive copy of the new replacement when set via > replacementWith() > > To return a defensive copy of replacement from replacement() may > trigger minor performance "regression" when internal interface > ArrayEncoder.encode() is invoked (from StringCoding and ZipCoder), > so some corresponding updates in UTF8, DoubleByte and HKSCS clases > to save copy of the replacement byte array or byte (SingleByte class > already makes its own copy). > > Thanks, > -Sherman > > >
