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