Hi Andrew,

 thanks for the review. I am pushing the change.

 Sorry for the mess.

Thanks,
Andrew

On 3/7/2013 7:12 PM, Andrew Hughes wrote:
----- 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




Reply via email to