Thanks Mandy, and all others have reviewed and commented. Cheers, Henry
On 10/09/2013 02:47 AM, Mandy Chung wrote: > Hi Henry, > > On 10/8/2013 10:57 PM, Henry Jen wrote: >> Hi, >> >> Please review updated webrev at >> http://cr.openjdk.java.net/~henryjen/ccc/8023524/6/webrev > > ProxyClassesDumper looks simpler after moving the path validation to the > static factory method. > > One minor comment: > ProxyClassesDumper.getInstance returns null if the given path is invalid. > For a null path, it can simply return null rather than throwing NPE. > > It's good to see the limited doPrivileged being used here. I can > see that listing the limited permissions would be a tradeoff with > maintainability and readability. This also leads to the question > how one can determine the complete list of permissions required > (besides testing) and guideline on when to use limited privileged > and when to use the entire ACC (Jeff may have some thoughts). > > Anyway, just some comments related to limited doPrivileged and not > anything related to this fix. > > Henry - your fix looks good for me. You don't not need to > generate a new webrev if you make any change per my comment about > null path unless others have any other comment. > > thanks > Mandy