[7u-dev] Request for approval for CRs 8007454, 7147084

2013-09-24 Thread Ivan Gerasimov

Hello!

We have a request to backport fix for 7147084.

First, it depends on the fix for 8007454, so the webrev includes it too.
Second, the fix had to be adjusted a bit.
- I had to manually replaced Java_java_lang_ProcessImpl_create() 
function body with the new version, as 'hg patch' could not handle 
automatically.
- I had to replace wide-char strings to regular char*, in all the calls 
to win32Error() function.


Combined webrev:
http://cr.openjdk.java.net/~igerasim/7147084/0/webrev/

BUGS:
http://bugs.sun.com/view_bug.do?bug_id=7147084
http://bugs.sun.com/view_bug.do?bug_id=8007454

JDK8 Changesets:
7147084: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/2c4f1081a0fa
8007454: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/0a2b308cc82d

Reviews for jdk8 fix:

7147084: 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-August/019604.html
8007454: 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-February/014602.html


I had set up a JPRT job, which completed successfully.
All the .*jdk_lang.* test passed.

Sincerely yours,
Ivan Gerasimov


Re: [7u-dev] Request for approval for CRs 8007454, 7147084

2013-09-24 Thread Ivan Gerasimov

It was meant to be sent to jdk7u-dev


On 24.09.2013 12:03, Ivan Gerasimov wrote:

Hello!

We have a request to backport fix for 7147084.

First, it depends on the fix for 8007454, so the webrev includes it too.
Second, the fix had to be adjusted a bit.
- I had to manually replaced Java_java_lang_ProcessImpl_create() 
function body with the new version, as 'hg patch' could not handle 
automatically.
- I had to replace wide-char strings to regular char*, in all the 
calls to win32Error() function.


Combined webrev:
http://cr.openjdk.java.net/~igerasim/7147084/0/webrev/

BUGS:
http://bugs.sun.com/view_bug.do?bug_id=7147084
http://bugs.sun.com/view_bug.do?bug_id=8007454

JDK8 Changesets:
7147084: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/2c4f1081a0fa
8007454: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/0a2b308cc82d

Reviews for jdk8 fix:

7147084: 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-August/019604.html
8007454: 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-February/014602.html


I had set up a JPRT job, which completed successfully.
All the .*jdk_lang.* test passed.

Sincerely yours,
Ivan Gerasimov






Re: JDK-8020981: Update methods of java.lang.reflect.Parameter to throw correct exceptions

2013-09-24 Thread Eric McCorkle
Updated webrev here:
http://cr.openjdk.java.net/~emc/8020981/

Are there any more comments, or is this good to go?

On 09/19/13 18:15, Eric McCorkle wrote:
 The webrev has been updated with Joe's comments addressed.
 
 On 09/19/13 00:11, David Holmes wrote:
 On 19/09/2013 9:59 AM, Eric McCorkle wrote:
 This still needs to be reviewed.

 You seem to have ignored Joe's comments regarding the change to Modifier
 being incorrect.

 David

 On 09/16/13 14:50, Eric McCorkle wrote:
 I pulled the class files into byte array constants, as a temporary
 measure until a viable method for testing bad class files is developed.

 The webrev has been refreshed.  The class files will be taken out before
 I push.

 http://cr.openjdk.java.net/~emc/8020981/

 On 09/13/13 20:48, Joe Darcy wrote:
 To avoid storing binaries in Hg, you could try something like:

 * uuencode / ascii armor the class file
 * convert to byte array in the test
 * load classes from byte array

 -Joe

 On 09/13/2013 11:54 AM, Eric McCorkle wrote:
 I did it by hand with emacs.

 I would really rather tackle the bad class files for testing issue
 once
 and for all, the Right Way (tm).  But with ZBB looming, now is not the
 time to do it.

 Hence, I have created this task
 https://bugs.openjdk.java.net/browse/JDK-8024674

 I also just created this one:
 https://bugs.openjdk.java.net/browse/JDK-8024812

 On 09/13/13 13:54, Peter Levart wrote:
 Hi Eric,

 How did you create those class files? By hand using a HEX editor? Did
 you create a program that patched the original class file? If the
 later
 is the case, you could pack that patching logic inside a custom
 ClassLoader...

 To hacky? Dependent on future changes of javac? At least the bad
 name
 patching could be performed trivially and reliably, I think:
 search and
 replace with same-length string.

 Regards, Peter

 On 09/13/2013 07:35 PM, Eric McCorkle wrote:
 Ugh.  Byte arrays of class file data is really a horrible solution.

 I have already filed a task for test development post ZBB to
 develop a
 solution for generating bad class files.  I'd prefer to file a
 follow-up
 to this to add the bad class file tests when that's done.

 On 09/13/13 10:55, Joel Borggrén-Franck wrote:
 I think the right thing to do is to include the original compiling
 source in a comment, together with a comment on how you modify
 them, and then the result as a byte array.

 IIRC I have seen test of that kind before somewhere in our repo.

 cheers
 /Joel

 On Sep 13, 2013, at 4:49 PM, Eric McCorkle
 eric.mccor...@oracle.com wrote:

 There is no simple means of generating bad class files for
 testing.
 This is a huge deficiency in our testing abilities.

 If these class files shouldn't go in, then I'm left with no choice
 but
 to check in no test for this patch.

 However, anyone can run the test I've provided with the class
 files and
 see that it works.

 On 09/13/13 09:55, Joel Borggrén-Franck wrote:
 Hi Eric,

 IIRC we don't check in classfiles into the repo.

 I'm not sure how we handle testing of broken class-files in jdk,
 but ASM might be an option, or storing the class file as an
 embedded byte array in the test.

 cheers
 /Joel

 On Sep 13, 2013, at 3:40 PM, Eric McCorkle
 eric.mccor...@oracle.com wrote:

 A new webrev is posted (and crucible updated), which actually
 validates
 parameter names correctly.  Apologies for the last one.

 On 09/12/13 16:02, Eric McCorkle wrote:
 Hello,

 Please review this patch, which implements correct behavior for
 the
 Parameter Reflection API in the case of malformed class files.

 The bug report is here:
 https://bugs.openjdk.java.net/browse/JDK-8020981

 The webrev is here:
 http://cr.openjdk.java.net/~emc/8020981/

 This review is also on crucible.  The ID is CR-JDK8TL-182.

 Thanks,
 Eric

 eric_mccorkle.vcf



Re: JDK-8020981: Update methods of java.lang.reflect.Parameter to throw correct exceptions

2013-09-24 Thread Joel Borggren-Franck
Hi Eric,

Some feedback:

Executable.java:

299  * (i) The number of parameters (parameter_count) is wrong for the 
method

What is wrong in this case? Do you mean inconsistent with the signature?

302  * (iv) A parameter's name is , or contains an illegal character [0]

What does [0] mean in this case? A placeholder for an in-mail reference
or a null byte in a modified utf8 array?

299  * (i) The number of parameters (parameter_count) is wrong for the 
method
300  * (ii) A constant pool index is out of bounds.
301  * (iii) A constant pool index does not refer to a UTF-8 entry
302  * (iv) A parameter's name is , or contains an illegal character [0]
303  * (v) The flags field contains an illegal flag (something other than
304  * FINAL, SYNTHETIC, or MANDATED)

The new markup looks odd. I think you should use ul or ol to be
consistent j.l.Class. See Class.getMethods() for an example.


306  * @throws MalformedParametersException if the class file contains
307  * a MethodParameters attribute that is improperly formatted.
308  * @return an array of {@code Parameter} objects representing all
309  * the parameters to the executable this object represents

@throws ends with a period, @return does not. I'm not sure what the
convention is but I think you want to be consistent.


364 try {
365 tmp = getParameters0();
366 } catch(IllegalArgumentException e) {
367 // Rethrow ClassFormatErrors
368 throw new MalformedParametersException(Invalid constant 
pool index);
369 }

What is causing the IAE? Have you considered having
MalformedParametersExcepton taking a Throwable cause and having the IAE
as cause?


MalformedParametersException.java:

42 /**
43  * Use serialVersionUID from JDK 1.1.X for interoperability
44  */
45 private static final long serialVersionUID = 20130919L;

Was this type present in 1.1.x? This comment makes no sense to me.
Please explain.


BadClassFiles.java:

Thanks for encoding the class-files. Please include the original source
above the bytes.

cheers
/Joel


On 2013-09-19, Eric McCorkle wrote:
 The webrev has been updated with Joe's comments addressed.
 
 On 09/19/13 00:11, David Holmes wrote:
  On 19/09/2013 9:59 AM, Eric McCorkle wrote:
  This still needs to be reviewed.
  
  You seem to have ignored Joe's comments regarding the change to Modifier
  being incorrect.
  
  David
  
  On 09/16/13 14:50, Eric McCorkle wrote:
  I pulled the class files into byte array constants, as a temporary
  measure until a viable method for testing bad class files is developed.
 
  The webrev has been refreshed.  The class files will be taken out before
  I push.
 
  http://cr.openjdk.java.net/~emc/8020981/
 
  On 09/13/13 20:48, Joe Darcy wrote:
  To avoid storing binaries in Hg, you could try something like:
 
  * uuencode / ascii armor the class file
  * convert to byte array in the test
  * load classes from byte array
 
  -Joe
 
  On 09/13/2013 11:54 AM, Eric McCorkle wrote:
  I did it by hand with emacs.
 
  I would really rather tackle the bad class files for testing issue
  once
  and for all, the Right Way (tm).  But with ZBB looming, now is not the
  time to do it.
 
  Hence, I have created this task
  https://bugs.openjdk.java.net/browse/JDK-8024674
 
  I also just created this one:
  https://bugs.openjdk.java.net/browse/JDK-8024812
 
  On 09/13/13 13:54, Peter Levart wrote:
  Hi Eric,
 
  How did you create those class files? By hand using a HEX editor? Did
  you create a program that patched the original class file? If the
  later
  is the case, you could pack that patching logic inside a custom
  ClassLoader...
 
  To hacky? Dependent on future changes of javac? At least the bad
  name
  patching could be performed trivially and reliably, I think:
  search and
  replace with same-length string.
 
  Regards, Peter
 
  On 09/13/2013 07:35 PM, Eric McCorkle wrote:
  Ugh.  Byte arrays of class file data is really a horrible solution.
 
  I have already filed a task for test development post ZBB to
  develop a
  solution for generating bad class files.  I'd prefer to file a
  follow-up
  to this to add the bad class file tests when that's done.
 
  On 09/13/13 10:55, Joel Borggrén-Franck wrote:
  I think the right thing to do is to include the original compiling
  source in a comment, together with a comment on how you modify
  them, and then the result as a byte array.
 
  IIRC I have seen test of that kind before somewhere in our repo.
 
  cheers
  /Joel
 
  On Sep 13, 2013, at 4:49 PM, Eric McCorkle
  eric.mccor...@oracle.com wrote:
 
  There is no simple means of generating bad class files for
  testing.
  This is a huge deficiency in our testing abilities.
 
  If these class files shouldn't go in, then I'm left with no choice
  but
  to check in no test for this patch.
 
  However, anyone can run the test I've provided with the class
  files and

hg: jdk8/tl/jdk: 8014659: NPG: performance counters for compressed klass space

2013-09-24 Thread stefan . karlsson
Changeset: b606775fd1a3
Author:stefank
Date:  2013-08-29 11:08 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/b606775fd1a3

8014659: NPG: performance counters for compressed klass space
Reviewed-by: jmasa, sla
Contributed-by: erik.he...@oracle.com

! src/share/classes/sun/tools/jstat/resources/jstat_options
! test/sun/tools/jstat/gcCapacityOutput1.awk
! test/sun/tools/jstat/gcCauseOutput1.awk
! test/sun/tools/jstat/gcMetaCapacityOutput1.awk
! test/sun/tools/jstat/gcOldOutput1.awk
! test/sun/tools/jstat/gcOutput1.awk
! test/sun/tools/jstat/lineCounts1.awk
! test/sun/tools/jstat/lineCounts2.awk
! test/sun/tools/jstat/lineCounts3.awk
! test/sun/tools/jstat/lineCounts4.awk
! test/sun/tools/jstat/timeStamp1.awk
! test/sun/tools/jstatd/jstatGcutilOutput1.awk



Re: RFR: 8025140 - TEST_BUG: java/util/logging/Logger/getGlobal tests fail due to timeout

2013-09-24 Thread Alan Bateman

On 24/09/2013 07:24, Daniel Fuchs wrote:

:

There doesn't seem to be any reason to use any magic timeout value in
these tests - so I simply removed the /timeout=10 option and verified
that the tests passed in configurations where they previously failed.
 Sometimes people specify /timeout when they are checking the test 
against a JDK without the fix and maybe this is the case here. In any 
case, I agree there isn't any need for it and your changes look okay to me.


-Alan


Overloads warnings overly agressive?

2013-09-24 Thread Paul Sandoz
Hi,

There is a new warning about overloads on methods with functional interfaces, 
but it appears to be over-agressive as Doug pointed out to me off-list.

If i enable this when compiling tl (make JAVAC_WARNINGS:=-Xlint:overloads) then 
one can observe warnings such as on the primitive spliterators:

/Users/sandoz/Projects/jdk8/tl/jdk/src/share/classes/java/util/Spliterator.java:642:
 warning: [overloads] tryAdvance(IntConsumer) in OfInt is potentially ambiguous 
with tryAdvance(Consumer? super Integer) in OfInt
boolean tryAdvance(IntConsumer action);
^
/Users/sandoz/Projects/jdk8/tl/jdk/src/share/classes/java/util/Spliterator.java:645:
 warning: [overloads] forEachRemaining(IntConsumer) in OfInt is potentially 
ambiguous with forEachRemaining(Consumer? super Integer) in OfInt
default void forEachRemaining(IntConsumer action) {

The warnings propagate down to implementations, for example:

/Users/sandoz/Projects/jdk8/tl/jdk/src/share/classes/java/util/Random.java:1025:
 warning: [overloads] tryAdvance(IntConsumer) in RandomIntsSpliterator is 
potentially ambiguous with tryAdvance(Consumer? super T) in Spliterator
public boolean tryAdvance(IntConsumer consumer) {
   ^
  where T is a type-variable:
T extends Object declared in interface Spliterator
/Users/sandoz/Projects/jdk8/tl/jdk/src/share/classes/java/util/Random.java:1036:
 warning: [overloads] forEachRemaining(IntConsumer) in RandomIntsSpliterator is 
potentially ambiguous with forEachRemaining(Consumer? super T) in Spliterator
public void forEachRemaining(IntConsumer consumer) {

(Incidentally it does not appear all warnings are reported, warnings for the 
double implementations are missing.)

If I write a SuppressWarnings on Spliterator.OfInt:

@SuppressWarnings(overloads)
public interface OfInt extends OfPrimitiveInteger, IntConsumer, OfInt {

then that stops the first set of warnings (above) but the warnings are still 
propagated to the second set for Random (or in general implementations or 
extensions of). That seems over aggressive and more of an annoyance than 
helpful. Can we change this?

Paul.


hg: jdk8/tl/langtools: 8025050: Doclint doesn't recognize dfn tag

2013-09-24 Thread jonathan . gibbons
Changeset: 96dcb66e6b0a
Author:jjg
Date:  2013-09-24 10:48 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/96dcb66e6b0a

8025050: Doclint doesn't recognize dfn tag
Reviewed-by: bpatel

! src/share/classes/com/sun/tools/doclint/HtmlTag.java
! test/tools/doclint/html/InlineTagsTest.java



hg: jdk8/tl/langtools: 8025246: [doclint] doclint is showing error on anchor already defined when it's not

2013-09-24 Thread jonathan . gibbons
Changeset: 503338f16d2b
Author:jjg
Date:  2013-09-24 10:51 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/503338f16d2b

8025246: [doclint] doclint is showing error on anchor already defined when it's 
not
Reviewed-by: bpatel

! src/share/classes/com/sun/tools/doclint/Checker.java
+ test/tools/doclint/anchorTests/p/Test.java
+ test/tools/doclint/anchorTests/p/Test.javac.out
+ test/tools/doclint/anchorTests/p/Test.out
+ test/tools/doclint/anchorTests/p/package-info.java
+ test/tools/doclint/anchorTests/p/package-info.javac.out
+ test/tools/doclint/anchorTests/p/package-info.out



Re: Overloads warnings overly agressive?

2013-09-24 Thread Henry Jen
Hi,

I had reported this issue with attached test with behavior I observed, Brian 
and me agreed that SuppressWarnings on either one as in this test case should 
be sufficient.

Cheers,
Henry




On Sep 24, 2013, at 10:14 AM, Paul Sandoz paul.san...@oracle.com wrote:

 Hi,
 
 There is a new warning about overloads on methods with functional interfaces, 
 but it appears to be over-agressive as Doug pointed out to me off-list.
 
 If i enable this when compiling tl (make JAVAC_WARNINGS:=-Xlint:overloads) 
 then one can observe warnings such as on the primitive spliterators:
 
 /Users/sandoz/Projects/jdk8/tl/jdk/src/share/classes/java/util/Spliterator.java:642:
  warning: [overloads] tryAdvance(IntConsumer) in OfInt is potentially 
 ambiguous with tryAdvance(Consumer? super Integer) in OfInt
boolean tryAdvance(IntConsumer action);
^
 /Users/sandoz/Projects/jdk8/tl/jdk/src/share/classes/java/util/Spliterator.java:645:
  warning: [overloads] forEachRemaining(IntConsumer) in OfInt is potentially 
 ambiguous with forEachRemaining(Consumer? super Integer) in OfInt
default void forEachRemaining(IntConsumer action) {
 
 The warnings propagate down to implementations, for example:
 
 /Users/sandoz/Projects/jdk8/tl/jdk/src/share/classes/java/util/Random.java:1025:
  warning: [overloads] tryAdvance(IntConsumer) in RandomIntsSpliterator is 
 potentially ambiguous with tryAdvance(Consumer? super T) in Spliterator
public boolean tryAdvance(IntConsumer consumer) {
   ^
  where T is a type-variable:
T extends Object declared in interface Spliterator
 /Users/sandoz/Projects/jdk8/tl/jdk/src/share/classes/java/util/Random.java:1036:
  warning: [overloads] forEachRemaining(IntConsumer) in RandomIntsSpliterator 
 is potentially ambiguous with forEachRemaining(Consumer? super T) in 
 Spliterator
public void forEachRemaining(IntConsumer consumer) {
 
 (Incidentally it does not appear all warnings are reported, warnings for the 
 double implementations are missing.)
 
 If I write a SuppressWarnings on Spliterator.OfInt:
 
@SuppressWarnings(overloads)
public interface OfInt extends OfPrimitiveInteger, IntConsumer, OfInt {
 
 then that stops the first set of warnings (above) but the warnings are still 
 propagated to the second set for Random (or in general implementations or 
 extensions of). That seems over aggressive and more of an annoyance than 
 helpful. Can we change this?
 
 Paul.



Re: Overloads warnings overly agressive?

2013-09-24 Thread Henry Jen
Forgot that attachment is not allowed, the test code is following,

 public class TestOverload {
 interface FooT {
 void m(T arg);
 }
 
 interface IntFoo {
 void m(int arg);
 }
 
 interface BarT {
 void bar(FooT arg);
 }
 
 interface IntBar extends BarInteger {
 @SuppressWarnings(overloads)
 default void bar(IntFoo arg) {
 System.out.print(Primitive: );
 }
 
 @SuppressWarnings(overloads)
 default void bar(FooInteger arg) {
 System.out.print(Boxed: );
 bar((IntFoo) arg::m);
 }
 }
 
 public static void main(String[] args) {
 class Impl implements IntBar {
 @Override
 public void bar(IntFoo foo) {
 IntBar.super.bar(foo);
 System.out.print(Overload: );
 for (String arg: args) {
 Integer n = Integer.valueOf(arg);
 foo.m(n.intValue());
 }
 }
 }
 Impl impl = new Impl();
 impl.bar((int n) - System.out.println(n + 100));
 impl.bar((Integer n) - System.out.println(n));
 }
 }


On Sep 24, 2013, at 11:11 AM, Henry Jen henry@oracle.com wrote:

 Hi,
 
 I had reported this issue with attached test with behavior I observed, Brian 
 and me agreed that SuppressWarnings on either one as in this test case should 
 be sufficient.
 
 Cheers,
 Henry
 



hg: jdk8/tl/langtools: 8025272: doclint needs to check for valid usage of @value tag

2013-09-24 Thread jonathan . gibbons
Changeset: 6a05a713450d
Author:jjg
Date:  2013-09-24 11:46 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/6a05a713450d

8025272: doclint needs to check for valid usage of @value tag
Reviewed-by: bpatel

! src/share/classes/com/sun/tools/doclint/Checker.java
! src/share/classes/com/sun/tools/doclint/resources/doclint.properties
+ test/tools/doclint/ValueTest.java
+ test/tools/doclint/ValueTest.out



Re: JDK-8020981: Update methods of java.lang.reflect.Parameter to throw correct exceptions

2013-09-24 Thread Eric McCorkle
Webrev updated to address these issues.

On 09/24/13 07:51, Joel Borggren-Franck wrote:

 364 try {
 365 tmp = getParameters0();
 366 } catch(IllegalArgumentException e) {
 367 // Rethrow ClassFormatErrors
 368 throw new MalformedParametersException(Invalid constant 
 pool index);
 369 }
 
 What is causing the IAE? Have you considered having
 MalformedParametersExcepton taking a Throwable cause and having the IAE
 as cause?
 

The IAE comes from hotspot itself.  There is a standard constant pool
index verifying function that throws IAE if given a bad index.

 On 2013-09-19, Eric McCorkle wrote:
 The webrev has been updated with Joe's comments addressed.

 On 09/19/13 00:11, David Holmes wrote:
 On 19/09/2013 9:59 AM, Eric McCorkle wrote:
 This still needs to be reviewed.

 You seem to have ignored Joe's comments regarding the change to Modifier
 being incorrect.

 David

 On 09/16/13 14:50, Eric McCorkle wrote:
 I pulled the class files into byte array constants, as a temporary
 measure until a viable method for testing bad class files is developed.

 The webrev has been refreshed.  The class files will be taken out before
 I push.

 http://cr.openjdk.java.net/~emc/8020981/

 On 09/13/13 20:48, Joe Darcy wrote:
 To avoid storing binaries in Hg, you could try something like:

 * uuencode / ascii armor the class file
 * convert to byte array in the test
 * load classes from byte array

 -Joe

 On 09/13/2013 11:54 AM, Eric McCorkle wrote:
 I did it by hand with emacs.

 I would really rather tackle the bad class files for testing issue
 once
 and for all, the Right Way (tm).  But with ZBB looming, now is not the
 time to do it.

 Hence, I have created this task
 https://bugs.openjdk.java.net/browse/JDK-8024674

 I also just created this one:
 https://bugs.openjdk.java.net/browse/JDK-8024812

 On 09/13/13 13:54, Peter Levart wrote:
 Hi Eric,

 How did you create those class files? By hand using a HEX editor? Did
 you create a program that patched the original class file? If the
 later
 is the case, you could pack that patching logic inside a custom
 ClassLoader...

 To hacky? Dependent on future changes of javac? At least the bad
 name
 patching could be performed trivially and reliably, I think:
 search and
 replace with same-length string.

 Regards, Peter

 On 09/13/2013 07:35 PM, Eric McCorkle wrote:
 Ugh.  Byte arrays of class file data is really a horrible solution.

 I have already filed a task for test development post ZBB to
 develop a
 solution for generating bad class files.  I'd prefer to file a
 follow-up
 to this to add the bad class file tests when that's done.

 On 09/13/13 10:55, Joel Borggrén-Franck wrote:
 I think the right thing to do is to include the original compiling
 source in a comment, together with a comment on how you modify
 them, and then the result as a byte array.

 IIRC I have seen test of that kind before somewhere in our repo.

 cheers
 /Joel

 On Sep 13, 2013, at 4:49 PM, Eric McCorkle
 eric.mccor...@oracle.com wrote:

 There is no simple means of generating bad class files for
 testing.
 This is a huge deficiency in our testing abilities.

 If these class files shouldn't go in, then I'm left with no choice
 but
 to check in no test for this patch.

 However, anyone can run the test I've provided with the class
 files and
 see that it works.

 On 09/13/13 09:55, Joel Borggrén-Franck wrote:
 Hi Eric,

 IIRC we don't check in classfiles into the repo.

 I'm not sure how we handle testing of broken class-files in jdk,
 but ASM might be an option, or storing the class file as an
 embedded byte array in the test.

 cheers
 /Joel

 On Sep 13, 2013, at 3:40 PM, Eric McCorkle
 eric.mccor...@oracle.com wrote:

 A new webrev is posted (and crucible updated), which actually
 validates
 parameter names correctly.  Apologies for the last one.

 On 09/12/13 16:02, Eric McCorkle wrote:
 Hello,

 Please review this patch, which implements correct behavior for
 the
 Parameter Reflection API in the case of malformed class files.

 The bug report is here:
 https://bugs.openjdk.java.net/browse/JDK-8020981

 The webrev is here:
 http://cr.openjdk.java.net/~emc/8020981/

 This review is also on crucible.  The ID is CR-JDK8TL-182.

 Thanks,
 Eric

 eric_mccorkle.vcf

 


hg: jdk8/tl/langtools: 8002154: [doclint] doclint should check for issues which are errors in javadoc

2013-09-24 Thread jonathan . gibbons
Changeset: 3ae62331a56f
Author:jjg
Date:  2013-09-24 13:48 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/3ae62331a56f

8002154: [doclint] doclint should check for issues which are errors in javadoc
Reviewed-by: bpatel

! src/share/classes/com/sun/tools/doclint/Checker.java
! src/share/classes/com/sun/tools/doclint/resources/doclint.properties
! test/tools/doclint/ReferenceTest.java
! test/tools/doclint/ReferenceTest.out



JDK-8020981: Update methods of java.lang.reflect.Parameter to throw correct exceptions

2013-09-24 Thread Paul Benedict
Eric,

Should MalformedParametersException save IAE as the root cause? Or is that
an internal detail you don't want leaked?

 Webrev updated to address these issues.

 On 09/24/13 07:51, Joel Borggren-Franck wrote:

  364 try {
  365 tmp = getParameters0();
  366 } catch(IllegalArgumentException e) {
  367 // Rethrow ClassFormatErrors
  368 throw new MalformedParametersException(Invalid
constant pool index);
  369 }
 
  What is causing the IAE? Have you considered having
  MalformedParametersExcepton taking a Throwable cause and having the IAE
  as cause?
 

 The IAE comes from hotspot itself.  There is a standard constant pool
 index verifying function that throws IAE if given a bad index.

Cheers,
Paul


Re: RFR: 8025140 - TEST_BUG: java/util/logging/Logger/getGlobal tests fail due to timeout

2013-09-24 Thread Mandy Chung

Thumbs up.

Mandy

On 9/24/2013 7:24 AM, Daniel Fuchs wrote:

Hi,

This is a trivial fix for:

8025140 - TEST_BUG: java/util/logging/Logger/getGlobal tests fail
  due to timeout

JBS:https://bugs.openjdk.java.net/browse/JDK-8025140
webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8025140/webrev.00/


The issue here is that the getGlobal/* tests had @run lines containing
timeout value which were too aggressive for certain configurations -
such as fastdebug JDK builds with all twist  bell options enabled.

There doesn't seem to be any reason to use any magic timeout value in
these tests - so I simply removed the /timeout=10 option and verified
that the tests passed in configurations where they previously failed.

best regards,

-- daniel




RFR (2nd): 8023524: Mechanism to dump generated lambda classes / log lambda code generation

2013-09-24 Thread Henry Jen
Hi,

Please review the new webrev at
http://cr.openjdk.java.net/~henryjen/ccc/8023524/1/webrev/

The updated code will attempt to escape potentially bad characters(based
on our best knowledge on Windows and common systems), it's not likely we
can avoid problem for all file systems.

Anyhow, we avoid characters that can be used to navigate (known) file
system, and if there is any other invalid characters, that should cause
an IOException failed to create file and just skip dumping of that class.

Let me know if there are other concerns.

Cheers,
Henry


On 09/19/2013 12:27 AM, Florian Weimer wrote:
 On 09/19/2013 01:00 AM, Henry Jen wrote:
 
 Class names can contain '\' and other characters which are problematic
 on Windows.
 
 Thanks for reviewing, I suspect you are pointing out a potential issue
 to look at, not that the problem exists in current implementation.

 According to JLS 3.8, the classname(an identifier) can only have
 letters, digits, plus '_' and '$'.
 
 You need to look at the JVM specification, there are only very few
 characters it excludes.  The restrictions come from javac, not the JVM.
  For example, on Linux, java '\' will load a \.class file and run it
 (yes, I tried).
 



hg: jdk8/tl/langtools: 8016328: Regression : Javadoc i18n regression caused by fix for 8012375

2013-09-24 Thread bhavesh . x . patel
Changeset: 184c0d6698c3
Author:bpatel
Date:  2013-09-24 16:12 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/184c0d6698c3

8016328: Regression : Javadoc i18n regression caused by fix for 8012375
Reviewed-by: jjg

! src/share/classes/com/sun/tools/doclets/formats/html/markup/HtmlTree.java
! src/share/classes/com/sun/tools/doclets/formats/html/markup/HtmlWriter.java
! test/com/sun/javadoc/testHref/TestHref.java
! test/com/sun/javadoc/testJavascript/TestJavascript.java
! test/com/sun/javadoc/testLinkTaglet/TestLinkTaglet.java
! test/com/sun/javadoc/testPrivateClasses/TestPrivateClasses.java
! test/com/sun/javadoc/testUseOption/TestUseOption.java



Re: RFR (2nd): 8023524: Mechanism to dump generated lambda classes / log lambda code generation

2013-09-24 Thread Peter Levart

On 09/24/2013 11:59 PM, Henry Jen wrote:

Hi,

Please review the new webrev at
http://cr.openjdk.java.net/~henryjen/ccc/8023524/1/webrev/

The updated code will attempt to escape potentially bad characters(based
on our best knowledge on Windows and common systems), it's not likely we
can avoid problem for all file systems.

Anyhow, we avoid characters that can be used to navigate (known) file
system, and if there is any other invalid characters, that should cause
an IOException failed to create file and just skip dumping of that class.

Let me know if there are other concerns.


Hi Henry,

Just a thought. How does URLClassLoader do the class name - path to 
resource translation? Perhaps there's already some code that does this 
correctly and in a platform-specific way (haven't looked)...


Regards, Peter



Cheers,
Henry


On 09/19/2013 12:27 AM, Florian Weimer wrote:

On 09/19/2013 01:00 AM, Henry Jen wrote:


Class names can contain '\' and other characters which are problematic
on Windows.

Thanks for reviewing, I suspect you are pointing out a potential issue
to look at, not that the problem exists in current implementation.

According to JLS 3.8, the classname(an identifier) can only have
letters, digits, plus '_' and '$'.

You need to look at the JVM specification, there are only very few
characters it excludes.  The restrictions come from javac, not the JVM.
  For example, on Linux, java '\' will load a \.class file and run it
(yes, I tried).