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

Andrew Brygin andrew.brygin at oracle.com
Thu Mar 7 06:04:51 PST 2013


Sorry, it was my mistake, I pushed changes without build failure correction.

May I push a fix for this into the same repo?

Thanks,
Andrew

On 3/7/2013 5:20 PM, Seán Coffey wrote:
> 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