Looks good to me (emcmanus). Éamonn
2012/10/12 Jaroslav Bachorik <[email protected]> > 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- > >>> > >>> > >> > >> > >
