I've updated the test, switched to an in-memory class loader, and added
a test case. Please review.
On 10/01/13 16:27, Eric McCorkle wrote:
On 10/01/13 02:41, Joe Darcy wrote:
(Suggested changes have been applied)
I think the test is acceptable as-is, but an RFE could be filed for some
Hi Eric,
Please revert the change to j.l.r.Modifer. The fix can be pushed with
just that modification; however, I strongly recommend also removing the
here is everything that can go wrong list from j.l.r.Executable. Core
reflection generally doesn't delve into such details in the main-line
Thanks, Joe. I reverted Modifier, and removed the list (I thought I had
done that already).
I will push after a successful test run.
On 10/02/13 15:54, Joe Darcy wrote:
Hi Eric,
Please revert the change to j.l.r.Modifer. The fix can be pushed with
just that modification; however, I
On 10/01/13 02:41, Joe Darcy wrote:
(Suggested changes have been applied)
I think the test is acceptable as-is, but an RFE could be filed for some
refactoring (having each bad class be represented as a diff from a base
byte[], avoiding sending the bytes through the file system).
Better
The enhanced metadata spec says nothing at all about an IAE. That is an
implementation detail, possibly subject to change at any point, and it
*should* not be leaked.
On 09/24/13 17:28, Paul Benedict wrote:
Eric,
Should MalformedParametersException save IAE as the root cause? Or is that
an
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
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
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
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
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
This still needs to be reviewed.
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
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
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
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
MalformedParametersException should receive a @since tag.
Additionally, the javadoc doesn't describe what it means for a parameter to
be malformed. The answer doesn't need to be exhaustive, but I think some
examples would help developers if they catch one and need to dig into class
files. Or if
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
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
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
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
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
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
On 09/13/13 09:53, Paul Benedict wrote:
MalformedParametersException should receive a @since tag.
Additionally, the javadoc doesn't describe what it means for a parameter to
be malformed. The answer doesn't need to be exhaustive, but I think some
examples would help developers if they catch
On 09/12/2013 01:02 PM, 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:
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
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
25 matches
Mail list logo
