Request for approval to sync Feb 2013 CPU fixes into jdk6-open

Seán Coffey sean.coffey at oracle.com
Thu Mar 7 05:20:36 PST 2013


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!

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



More information about the jdk6-dev mailing list