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

Reply via email to