Thanks. Minor nits picked ... http://btrace.kenai.com/webrevs/JDK-6783290/webrev.v3/
-JB- On 10/11/2012 07:42 PM, Eamonn McManus wrote: > Looks good. A couple of minor nits about the test: there is a stray > IDE template comment on line 74, and the copyright date is wrong. > > Éamonn > > > 2012/10/11 Jaroslav Bachorik <[email protected]>: >> Just to keep it clear - here is the webrev hosted at CR - >> http://cr.openjdk.java.net/~dsamersoff/sponsorship/jbachorik/JDK-6783290-v1/ >> >> -JB- >> >> On Wed 10 Oct 2012 08:46:04 PM CEST, Jaroslav Bachorik wrote: >>> Hi, >>> >>> On Wed 10 Oct 2012 05:49:11 PM CEST, Eamonn McManus wrote: >>>> Hi Jaroslav, >>>> >>>> The patch looks correct and the test is ingenious. >>>> >>>> I do not understand why the previous SerializationTest needs to be >>>> deleted. It doesn't seem that the new test is covering the same >>>> things. >>> >>> I need to check that. I copied the SerializationTest.java to >>> SerializationTest1.java - apparently the cooperation of the webrev and >>> mercurial queues has its glitches :( >>> >>> I am attaching the corrected webrev. >>> >>> -JB- >>> >>>> >>>> Reviewed-by: emcmanus >>>> >>>> Incidentally I was not able to find a way to see the patch with the >>>> usual webrev browser UI. Is there a link for that? >>>> >>>> Regards, >>>> Éamonn >>>> >>>> >>>> 2012/10/10 Jaroslav Bachorik <[email protected]>: >>>>> I am looking for a review and a sponsor for this fix. >>>>> >>>>> The issue is about an empty array of descriptors being written as a part >>>>> of the serialization process but not read when deserializing an >>>>> MBeanInfo/MBeanFeatureInfo instance. While the current ObjectInputStream >>>>> skips all unread custom written fields it is not a behaviour required by >>>>> the specification and may cause problems. >>>>> >>>>> The patch makes the array to be read in all cases - even when it is >>>>> known to be an empty one. That way all that has been written as a part >>>>> of serialization is read back. >>>>> >>>>> The webrev with the fix and test is available @ >>>>> https://github.com/jbachorik/openjdk-patches/tree/master/webrevs/JDK-6783290 >>>>> >>>>> -JB- >>> >>> >> >>
