<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
Erik, <br>
<br>
comments inline..<br>
<div class="moz-cite-prefix">On 08/11/18 15:12, Erik Gahlin wrote:<br>
</div>
<blockquote cite="mid:5BE45246.3050603@oracle.com" type="cite">
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
<div class="moz-cite-prefix">Hi Sean, <br>
<br>
I think we could still call the event
"jdk.SecurityPropertyModification", but add a @Description that
explains that events are only emitted for the JDK due to
security concerns. If we at a later stage want to include user
events, we could add those and remove the @Description, possibly
with a setting where you can specify scope, or perhaps a new
event all together.<br>
</div>
</blockquote>
sounds fine. I've made those changes.<br>
<blockquote cite="mid:5BE45246.3050603@oracle.com" type="cite">
<div class="moz-cite-prefix"> <br>
Perhaps we could find a better name for <span class="new">the
field validationEventNumber? It sounds like we have an event
number, which is not really the case. We have a counter for
the validation id.<br>
</span></div>
</blockquote>
How about 'validationCounter' ?<br>
<blockquote cite="mid:5BE45246.3050603@oracle.com" type="cite">
<div class="moz-cite-prefix"><span class="new"> <br>
I noticed that you use hashCode as an id. I assume this is to
simplify the implementation? That is probably OK, the risk of
a collision is perhaps low, but could you make the field a
long, so we in the future could add something more unique
(Flight Recorder uses compressed integers, so a long uses the
same amount of disk space as an int). Could you also rename
the field and the annotation, perhaps certificationId could
work, so we don't leak how the id was implemented. <br>
</span></div>
</blockquote>
Yes - originally, I was using the certificate serial number but that
may not always be unique (esp. for test generated certs). I've
changed the variable name to 'certificateId' and made it a long.
Renamed the annotation also.<br>
<blockquote cite="mid:5BE45246.3050603@oracle.com" type="cite">
<div class="moz-cite-prefix"><span class="new"> <br>
I could not find the file: test/lib/jdk/test/lib/security/</span>TestJdkSecurityPropertyModification.java<span
class="new"><br>
</span></div>
</blockquote>
whoops - forgot to add it to mercurial tracking. there now : <br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~coffeys/webrev.8148188.v8/webrev/">http://cr.openjdk.java.net/~coffeys/webrev.8148188.v8/webrev/</a><br>
<br>
regards,<br>
Sean.<br>
</body>
</html>