On Mon, 10 Oct 2022 07:19:30 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> Sean Coffey has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Check for 0 security events
>
> src/jdk.jfr/share/classes/jdk/jfr/events/InitialSecurityPropertyEvent.java
> line 29:
>
>> 27:
>> 28: import jdk.jfr.*;
>> 29: import jdk.jfr.internal.MirrorEvent;
>
> Hello Sean, this `MirrorEvent` appears to be an unused import. Furthermore,
> as far as I know, in `core-libs` the `*` wildcard imports aren't typically
> used. I don't know if it's OK to use it here in the `jdk.jfr` module.
Thanks Jai - I removed the unused import and reverted to non-wildcard import use
> src/jdk.jfr/share/classes/jdk/jfr/internal/instrument/JDKEvents.java line 318:
>
>> 316: InitialSecurityPropertyEvent e = new
>> InitialSecurityPropertyEvent();
>> 317: e.key = (String) entry.getKey();
>> 318: e.value = (String) entry.getValue();
>
> To avoid any (odd/unexpected) `ClassCastException` here, perhaps this loop
> can be changed to something like:
>
> for (Set<String> name : p.stringPropertyNames()) {
> InitialSecurityPropertyEvent e = new InitialSecurityPropertyEvent();
> e.key = name;
> e.value = p.getProperty(name);
>
> Since this code anyway wants to deal with string key/values, this wouldn't
> introduce any functional change and yet at the same time prevent any
> unexpected/theoretical `ClassCastException`s
nice - wasn't aware of that method in Properties. Code updated.
-------------
PR: https://git.openjdk.org/jdk/pull/10394
