Hi Harsha,
This looks reasonable. Some comments:
sun/management/Agent.java
567 ServiceLoader<AgentProvider> providers =
ServiceLoader.load(AgentProvider.class);
It should call ServiceLoader::loadInstalled since it should only load the
providers from the platform modules. You can also simplify line 567-570 to:
for (AgentProvider provider :
ServiceLoader.loadInstalled(AgentProvider.class)) {
:
}
sun/management/spi/AgentProvider.java
78 public abstract void startAgent(String port, Properties props);
The port parameter should be “int”.
There are a few references to “Agent” that should be “agent”, as it’s not a
type name.
Mandy
> On Aug 21, 2016, at 6:51 PM, Harsha Wardhana B <[email protected]>
> wrote:
>
> Hello All,
>
> Please find revised webrev located at,
>
> http://cr.openjdk.java.net/~hb/8131061/webrev.01/
>
> The new patch has below changes.
>
> 1. AgentProvider is made abstract class.
> 2. Loading SNMP Agent provider is done in a AccessController.doPreviliged
> block.
>
> Thanks
> Harsha
>
> On Friday 12 August 2016 10:31 AM, Harsha Wardhana B wrote:
>> Hi All,
>>
>> Please review fix for issue,
>>
>> JDK-8131061 - Use of -Dcom.sun.management.snmp needs to be examined for
>> modules
>> with webrev located at,
>>
>> http://cr.openjdk.java.net/~hb/8131061/webrev.00/
>>
>> Regards
>>
>> Harsha
>
