7192275: Minimize LogManager dependencies on java.beans

Alan Bateman Alan.Bateman at oracle.com
Sat Aug 18 12:37:20 PDT 2012


Thanks for reviewing this, replies inline.

On 18/08/2012 13:56, Dmitry Samersoff wrote:
> Alan,
>
> 1. 315 - IMHO, it's better to call checkAccess() before null pointer
> check. This problem exists in original code as well.
If there are two or more exceptions that are appropriate for a 
particular case then developers have to prepared for either. In the case 
if someone invokes the addPropertyChangeListener with null and at the 
same time would fail the permission check then either exception is 
valid; you can't assume one or the other. There can of course be cases 
where you have to be careful to do a permission check before other 
checks because failing some other check might reveal something to an 
adversary. This isn't the case here and in addition I don't think we 
should change existing behavior without good reason.

>
> 2. This place is not clean for me - env is constant under loop.
> is it intentional?
>
>   975                 for (int i = 0; i<  count; i++) {
>   976                     listener.propertyChange(ev);
>   977                 }
>
It is intentional as the spec requires that a listener be invoked for as 
many times as it is registered. I could of course have used a while loop 
instead, maybe "while (count-- > 0)" but I think the above is clear.

-Alan


More information about the serviceability-dev mailing list