----- Original Message ----- > Hello, > > please review a fix for the build failure: > > http://cr.openjdk.java.net/~bae/8009641/webrev.00/ >
Looks good to me and better than what I had as the synchronized blocks were still outside the try (so the variables could end up being null). Ok to commit. > Thanks, > Andrew > > On 3/7/2013 5:55 PM, Seán Coffey wrote: > > Even better if you want to push the change ? I haven't heard from > > Andrew B. yet. > > > > I've logged 8009641 to track this. You could use that ID if you > > want. > > > > 8009641: OpenJDK 6 build broken via 8007675 fix > > > > regards, > > Sean. > > > > On 07/03/2013 13:51, Andrew Hughes wrote: > >> > >> ----- Original Message ----- > >>> Yes - you're right. That does look like an issue. Andrew Brygin > >>> ran > >>> pre > >>> integration tests before pushing the changes internally and they > >>> were > >>> successful. However - I've traced back over the sources and what > >>> was > >>> run > >>> in his test build and what he pushed to internal repo differs. ( > >>> in 2 > >>> areas) - No doubt, it's the curse of the last minute change. > >>> > >>> Andrew Brygin, can you log a bug and get this corrected asap ? > >>> > >>> Thanks for the notice Andrew! > >>> > >> I have a patch I can post if that would help. With that, this > >> builds. > >> I just need to clean up my workspace and post a webrev. > >> > >>> Thanks, > >>> Sean. > >>> > >>> > >>>> public short[] colorConvert(short[] src, short[] dst) { > >>>> > >>>> if (dst == null) { > >>>> dst = new short > >>>> [(src.length/getNumInComponents())*getNumOutComponents()]; > >>>> } > >>>> > >>>> try { > >>>> LCMSImageLayout srcIL = new LCMSImageLayout( > >>>> src, src.length/getNumInComponents(), > >>>> LCMSImageLayout.CHANNELS_SH(getNumInComponents()) | > >>>> LCMSImageLayout.BYTES_SH(2), > >>>> getNumInComponents()*2); > >>>> > >>>> LCMSImageLayout dstIL = new LCMSImageLayout( > >>>> dst, dst.length/getNumOutComponents(), > >>>> LCMSImageLayout.CHANNELS_SH(getNumOutComponents()) > >>>> | > >>>> LCMSImageLayout.BYTES_SH(2), > >>>> getNumOutComponents()*2); > >>>> > >>>> synchronized(this) { > >>>> LCMS.colorConvert(this, srcIL, dstIL); > >>>> } > >>>> } catch (ImageLayoutException e) { > >>>> throw new CMMException("Unable to convert data"); > >>>> } > >>>> > >>>> return dst; > >>>> } > >>> On 07/03/2013 12:39, Andrew Hughes wrote: > >>>> ----- Original Message ----- > >>>>> ----- Original Message ----- > >>>>>> I'm only the proxy here but I created the webrev from the > >>>>>> changesets > >>>>>> that I pushed. I don't see any difference. > >>>>>> > >>>>>> t4 $diff LCMSTransform.java.webrev > >>>>>> jdk/src/share/classes/sun/java2d/cmm/lcms/LCMSTransform.java > >>>>>> t4 $ > >>>>>> > >>>>> Indeed. I'm still seeing the same failure using the changesets > >>>>> pushed > >>>>> to OpenJDK6, yet the repository itself builds. Very odd. > >>>>> > >>>> The copy of OpenJDK6 I was looking at wasn't up-to-date, hence > >>>> the > >>>> diff. > >>>> The patched version and upstream repo are identical when > >>>> compared > >>>> correctly. > >>>> > >>>> What's baffling is how this code compiles with the javac > >>>> compiler > >>>> as it looks > >>>> clearly wrong to me, which is why ecj falls over on it. > >>>> > >>>> try { > >>>> LCMSImageLayout srcIL = new LCMSImageLayout( > >>>> src, src.length/getNumInComponents(), > >>>> LCMSImageLayout.CHANNELS_SH(getNumInComponents()) > >>>> | > >>>> LCMSImageLayout.BYTES_SH(2), > >>>> getNumInComponents()*2); > >>>> > >>>> LCMSImageLayout dstIL = new LCMSImageLayout( > >>>> dst, dst.length/getNumOutComponents(), > >>>> LCMSImageLayout.CHANNELS_SH(getNumOutComponents()) > >>>> | > >>>> LCMSImageLayout.BYTES_SH(2), > >>>> getNumOutComponents()*2); > >>>> } catch (ImageLayoutException e) { > >>>> throw new CMMException("Unable to convert data"); > >>>> } > >>>> > >>>> synchronized(this) { > >>>> LCMS.colorConvert(this, srcIL, dstIL); > >>>> } > >>>> > >>>> srcIL and dstIL are declared inside the try block so aren't > >>>> visible > >>>> outside it. > >>>> > >>>> In the other colorConvert methods, these are declared at the top > >>>> of > >>>> the method. > >>>> > >>>>>> regards, > >>>>>> Sean. > >>>>>> > >>>>>> On 07/03/2013 10:59, Andrew Hughes wrote: > >>>>>>> ----- Original Message ----- > >>>>>>>> February CPU pushes completed as reviewed in last round of > >>>>>>>> webrevs. > >>>>>>>> > >>>>>>>> I'd like to push 2 extra fixes now for issues addressed in > >>>>>>>> yesterday's > >>>>>>>> JDK releases. > >>>>>>>> > >>>>>>>> webrev : > >>>>>>>> http://cr.openjdk.java.net/~coffeys/webrev.6open.mar5/ > >>>>>>>> > >>>>>>>> Good to push ? > >>>>>>>> > >>>>>>>> regards, > >>>>>>>> Sean. > >>>>>>>> > >>>>>>>> On 05/03/2013 18:44, Omair Majid wrote: > >>>>>>>>> On 03/05/2013 10:52 AM, Edvard Wendelin wrote: > >>>>>>>>>> Hi, > >>>>>>>>>> > >>>>>>>>>> The change in JAXP can be viewed here: > >>>>>>>>>> http://cr.openjdk.java.net/~joehw/jdk8/8001235/webrev/ > >>>>>>>>>> While > >>>>>>>>>> it's > >>>>>>>>>> generated against JDK 8, the change in 6 is identical. > >>>>>>>>> The JAXP changes look identical to what was pushed to > >>>>>>>>> jdk7u. > >>>>>>>>> Looks > >>>>>>>>> all > >>>>>>>>> good to me! > >>>>>>>>> > >>>>>>>>>> I plan to push the changes today. > >>>>>>>>> Thank you. It will be great to have all the security fixes > >>>>>>>>> in! > >>>>>>>>> > >>>>>>>>> Cheers, > >>>>>>>>> Omair > >>>>>>>>> > >>>>>>> I notice that what was finally committed differs from this > >>>>>>> webrev > >>>>>>> (in a positive > >>>>>>> way). The version posted from the webrev failed to compile: > >>>>>>> > >>>>>>> 1. ERROR in > >>>>>>> ../../../src/share/classes/sun/java2d/cmm/lcms/LCMSTransform.java > >>>>>>> (at line 580) > >>>>>>> LCMS.colorConvert(this, srcIL, dstIL); > >>>>>>> ^^^^^ > >>>>>>> srcIL cannot be resolved to a variable > >>>>>>> ---------- > >>>>>>> 2. ERROR in > >>>>>>> ../../../src/share/classes/sun/java2d/cmm/lcms/LCMSTransform.java > >>>>>>> (at line 580) > >>>>>>> LCMS.colorConvert(this, srcIL, dstIL); > >>>>>>> ^^^^^ > >>>>>>> dstIL cannot be resolved to a variable > >>>>>>> ---------- > >>>>>>> 3. ERROR in > >>>>>>> ../../../src/share/classes/sun/java2d/cmm/lcms/LCMSTransform.java > >>>>>>> (at line 606) > >>>>>>> LCMS.colorConvert(this, srcIL, dstIL); > >>>>>>> ^^^^^ > >>>>>>> srcIL cannot be resolved to a variable > >>>>>>> ---------- > >>>>>>> 4. ERROR in > >>>>>>> ../../../src/share/classes/sun/java2d/cmm/lcms/LCMSTransform.java > >>>>>>> (at line 606) > >>>>>>> LCMS.colorConvert(this, srcIL, dstIL); > >>>>>>> ^^^^^ > >>>>>>> dstIL cannot be resolved to a variable > >>>>>>> > >>>>>>> but this seems to have been fixed in the committed version, > >>>>>>> presumably due to: > >>>>>>> > >>>>>>> - try { > >>>>>>> - LCMSImageLayout srcIL = new LCMSImageLayout( > >>>>>>> - src, src.length/getNumInComponents(), > >>>>>>> - > >>>>>>> LCMSImageLayout.CHANNELS_SH(getNumInComponents()) > >>>>>>> | > >>>>>>> - LCMSImageLayout.BYTES_SH(1), > >>>>>>> getNumInComponents()); > >>>>>>> - > >>>>>>> - LCMSImageLayout dstIL = new LCMSImageLayout( > >>>>>>> - dst, dst.length/getNumOutComponents(), > >>>>>>> - > >>>>>>> LCMSImageLayout.CHANNELS_SH(getNumOutComponents()) > >>>>>>> | > >>>>>>> - LCMSImageLayout.BYTES_SH(1), > >>>>>>> getNumOutComponents()); > >>>>>>> - } catch (ImageLayoutException e) { > >>>>>>> - throw new CMMException("Unable to convert > >>>>>>> data"); > >>>>>>> - } > >>>>>>> + LCMSImageLayout srcIL = new LCMSImageLayout( > >>>>>>> + src, src.length/getNumInComponents(), > >>>>>>> + LCMSImageLayout.CHANNELS_SH(getNumInComponents()) | > >>>>>>> + LCMSImageLayout.BYTES_SH(1), > >>>>>>> getNumInComponents()); > >>>>>>> + > >>>>>>> + LCMSImageLayout dstIL = new LCMSImageLayout( > >>>>>>> + dst, dst.length/getNumOutComponents(), > >>>>>>> + LCMSImageLayout.CHANNELS_SH(getNumOutComponents()) > >>>>>>> | > >>>>>>> + LCMSImageLayout.BYTES_SH(1), > >>>>>>> getNumOutComponents()); > >>>>>>> > >>>>>>> so srcIL and dstIL are now declared at a wider scope. I'll > >>>>>>> use > >>>>>>> the > >>>>>>> committed versions > >>>>>>> in IcedTea6 HEAD. > >>>>>>> > >>>>>>> Cheers, > >>>>> -- > >>>>> Andrew :) > >>>>> > >>>>> Free Java Software Engineer > >>>>> Red Hat, Inc. (http://www.redhat.com) > >>>>> > >>>>> PGP Key: 248BDC07 (https://keys.indymedia.org/) > >>>>> Fingerprint = EC5A 1F5E C0AD 1D15 8F1F 8F91 3B96 A578 248B > >>>>> DC07 > >>>>> > >>>>> > >>> > > > > -- Andrew :) Free Java Software Engineer Red Hat, Inc. (http://www.redhat.com) PGP Key: 248BDC07 (https://keys.indymedia.org/) Fingerprint = EC5A 1F5E C0AD 1D15 8F1F 8F91 3B96 A578 248B DC07