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- >> >> > >
