On 23/06/2016 9:33 PM, serguei.spit...@oracle.com wrote:
On 6/23/16 04:27, David Holmes wrote:
On 23/06/2016 9:08 PM, serguei.spit...@oracle.com wrote:
On 6/23/16 03:51, David Holmes wrote:
On 23/06/2016 6:04 PM, serguei.spit...@oracle.com wrote:
On 6/23/16 00:51, Alan Bateman wrote:


On 23/06/2016 00:20, serguei.spit...@oracle.com wrote:
:

I agree with it.
Thank you for pointing to this JVMTI example.
I did not find in the JNI where the names are checked to be legal.
We are going to open a can of worms with this kind of check as there
can be many corner cases to cover.
The primary use-case for this function is from within the CLFH
callback so have it return the unnamed module when calling with
garage
is probably okay, it just needs to be specified.

Will you update the webrev to reflect where we've got to this in this
discussion?

I need to undo my fix for additional check.
My up-to-date understanding is that we have to explicitly specify two
points:
  - the function returns the unnamed module if the package does not
exist
  - the function does not check the package names for validness and
always returns the unnamed module for illegal package names

It is better to specify these points explicitly to avoid possible
confusion.

I don't agree - nothing else refers to invalid "names" in JVM TI. I
don't see any need to call this out here. If the name supplied is not
a legal package name then it will not match - end of story.

David,

It was the original approach that is in the CCC.
You were the one who got confused here, and it convinced me that this
needs to be more clear.
All these changes are because you started the discussion. :)
It was useful anyway.

I never said anything about illegal package names.

True.
This is my (probably, wrong) attempt to make it more clear.


Are you against both clarifications or just the last one?
I'm Ok to return it back as it is in the CCC.
It will save time on the CCC approval.

This is the last addition to the spec:

+ The unnamed module is returned if the specified package does not
exist.

The notion of "package does not exist" is ill-defined. This case is
already covered by the primary specification.

+ The function does not check if the specified package name is illegal.

This does not need to be stated as it is not stated anywhere else for
anything else that may have legal and illegal forms.

Good.
This is a relevant fragment from current version of CCC:

+      <description>
+        Return the <code>java.lang.reflect.Module</code> object for a
module
+        defined to a class loader that contains a given package.
+        The module is returned via <code>module_ptr</code>.
+        <p/>
+        If a named module is defined to the class loader and it
+        contains the package then that named module is returned,
+        otherwise the unnamed module of the class loader is returned.
+        If the package name is the empty string then this function
+        always returns the unnamed module for the class loader.
+        <p/>

As Stanislav said explicitly mentioning the empty string is not really necessary - but I don't see it as harmful.

Does it looks Ok?

Yes - as good now as it was in the CCC discussion :)

Cheers,
David


Thanks,
Serguei


Thanks,
David



Thanks,
Serguei





Cheers,
David


Thanks,
Serguei




-Alan



Reply via email to