Request for approval to sync Feb 2013 CPU fixes into jdk6-open
Andrew Hughes
gnu.andrew at redhat.com
Thu Mar 7 05:51:21 PST 2013
----- 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
More information about the jdk6-dev
mailing list