On 6/29/16 19:07, Daniel D. Daugherty wrote:
On 6/29/16 6:21 PM, [email protected] wrote:
Hi Dan,
On 6/29/16 07:59, Daniel D. Daugherty wrote:
On 6/29/16 2:53 AM, [email protected] wrote:
On 6/29/16 01:45, Jesper Wilhelmsson wrote:
29 juni 2016 kl. 04:30 skrev David Holmes <[email protected]>:
On 29/06/2016 12:09 PM, [email protected] wrote:
On 6/28/16 18:44, David Holmes wrote:
On 29/06/2016 7:09 AM, [email protected] wrote:
On 6/28/16 14:02, Daniel D. Daugherty wrote:
On 6/28/16 2:11 PM, [email protected] wrote:
On 6/28/16 11:19, Daniel D. Daugherty wrote:
I'll have to check the upper layers of this API,
but if an
agent can pass a bad 'class_loader' parameter and
get this
assert() to fire, then that's not good. Hopefully a
bad
'class_loader' parameter is caught at a higher layer.
Not sure, what do you mean.
Do you mean the generated JVMTI upper layer or the
JvmtiEnv::GetNamedModule?
Probably, the generated code.
I did mean the generated layer.
Ok, thanks.
Update: Yes, passing a non-NULL jobject as the
class_loader
parameter
when the jobject does not refer to a "class
loader" is
caught
at the upper layer.
The upper layer does not check that it is a class loader,
just for
non-NULL.
I think, it is good to have an assert here to double-checks the
pre-conditions as other caller can be added later.
But I'm Ok to get rid of it if you suggest.
Please keep the asserts. Basically I was mumbling to myself to
make sure that the asserts could not be reached by user code
and the "Update:" was to indicate that I did do.
Ok, thanks.
src/share/vm/prims/jvmti.xml
L6550: <param id="module_ptr">
L6551: <outptr><jobject/></outptr>
L6552: <description>
L6553: On return, points to a
<code>java.lang.reflect.Module</code> object.
L6554: </description>
L6555: </param>
The above wording doesn't allow for module_ptr to
be NULL
which
is a mismatch with the description.
I disagree (or maybe I got it incorrectly).
Pointing to NULL and be NULL is different.
It is not allowed for the module_ptr to be NULL but Ok to
pint to
NULL on return.
I think the description needs to be:
On return, points to a
<code>java.lang.reflect.Module</code> object
or points to a <code>NULL</code>.
Agreed, fixed.
Disagree. You would say that a pointer is NULL, not that it
points to
a NULL.
Why are you disagree?
The module_ptr is an out argument, it is not allowed to be NULL.
But the returning value by this pointer can be NULL.
As per later email I see this terminology already exists so is
fine, but I find it confusing to say "points to a NULL" - a NULL
is not an entity. If "foo points to a NULL" that implies to me
"foo == &NULL;" which is nonsense - foo == NULL, and if foo==NULL
then foo points to nothing. But the "pointer to a pointer" aspect
here does confuse things.
I would prefer if it said "points to a NULL pointer", or
NULL-pointer. I'm not sure what would be the more correct way to
write it. Anyway, a NULL pointer is an entity :-)
Jesper,
Thank you for the comment.
In fact, I've just used a pattern that is already present in the
JVMTI spec.
Objections to the existing pattern are minor, so it is better to be
more conservative here.
Perhaps we can use the wording in this JVM/TI function as a model:
http://docs.oracle.com/javase/8/docs/platform/jvmti/jvmti.html#GetCurrentContendedMonitor
This function takes a "jobject *" as a parameter and returns a
monitor pointer via that parameter. Pretty similar to what we're
discussing...
So here's the existing wording example:
Name Type Description
=========== ======== ===========
monitor_ptr jobject* On return, filled with the current contended
monitor,
or NULL if there is none.
Agent passes a pointer to a jobject. On
return, the
jobject has been set. The object returned by
monitor_ptr
is a JNI local reference and must be managed.
So for the new function:
module_ptr jobject* On return, filled with a
<code>java.lang.reflect.Module</code>
object or NULL if there is none.
This "filled with" style is used for return parameters in
~13 places in the JVM/TI spec.
Thank you for pointing to the example.
This discussion deviated to the question what the JVMTI pattern is
better to copy as a best practice.
Of course, now that I've found the GetCurrentContendedMonitor
example, That second paragraph or something like it is also
going to be needed with our new function...
Just wanted to note that this paragraph is not strictly necessary as
it is already covered in the common part of the spec:
http://docs.oracle.com/javase/8/docs/platform/jvmti/jvmti.html#refs
However, the second sentence is auto-generated from the jvmti.xml:
"Agent passes a pointer to a |jobject|. On return, the |jobject|
has been set.
The object returned by |module_ptr| is a JNI local reference and must
be managed."
Please, find the specdiff in the attachment.
The specdiff looks good.
There is one strange sentence:
> If class_loader is NULL, the bootstrap loader.
Perhaps "the bootstrap loader is assumed."?
Your call.
Interesting that a part of this sentence is auto-generated.
Fixed it, thanks!
Thanks,
Serguei
Dan
Thanks,
Serguei
Spec wording is very difficult... :-)
Dan
Thanks,
Serguei
/Jesper
Thanks,
David
Thanks,
Serguei
David
Thanks,
Serguei
Dan
