[foreign] RFR 8213343: Update the public access mode API

Jorn Vernee jbvernee at xs4all.nl
Wed Nov 21 02:35:56 UTC 2018


Ok, I've found one more mistake.

I made this patch a while ago already, and ran the tests to see if it 
still worked. However I seem to have made a mistake in code that is not 
being tested on windows (DirectSignatureShuffler). It seems obvious now, 
but I guess I missed it a while back.

Updated webrev: 
http://cr.openjdk.java.net/~jvernee/panama/webrevs/accessmode/webrev.03/

I've reverted the BoundedPointer fields back to being public. Making 
them private wasn't as simple of a change as I thought.

Sorry about that,
Jorn


Jorn Vernee schreef op 2018-11-21 00:22:
> And I suppose it's useful if I include the actual AccessMode class in
> the webrev as well (oops)
> 
> Webrev: 
> http://cr.openjdk.java.net/~jvernee/panama/webrevs/accessmode/webrev.01/
> 
> Cheers,
> Jorn
> 
> Jorn Vernee schreef op 2018-11-21 00:14:
>> Hi,
>> 
>> While reading through the Pointer class, I noticed there is a method
>> `isAccessibleFor(int)` which is used to check if the pointer is
>> accessible for a certain access mode.
>> 
>> The problem is that the `int` value that's expected as an argument is
>> defined as the constants MODE_R, MODE_W and MODE_RW in
>> BoundedMemoryRegion [1]. This is in an internal package, so there will
>> be no way for the user to use those constants.
>> 
>> The other problem is that having an `int` as the argument type gives
>> no hint to what values can be used there. I would like to advocate for
>> switching to an enum there, since then it's a simple case of typing in
>> the enum name, hitting the auto-complete key and getting a list of
>> available argument values.
>> 
>> The third problem is that there seems to be no public API point for
>> setting the access mode of a Pointer. I can imagine that somebody
>> might want to give out a read-only pointer to a client (and maybe even
>> a write-only pointer).
>> 
>> I have a patch that attempts to solve all 3 problems;
>> 
>> 1 & 2: I have created an AccessMode enum in j.f.memory that is used in
>> place of the `int`s.
>> 3: I have added 2 methods to Pointer, asReadOnly and asWriteOnly, to
>> create a new pointer from the given one with either of the 2 access
>> modes.
>> 
>> As an additional cleanup I've also removed the mode field from
>> BoundedPointer, since it was unused, and made the other fields private
>> instead of public. In order to do the latter, I added
>> putLongBits/getLongBits methods to BoundedPointer, which replaces
>> direct access to the pointer's region put/get methods. I also added a
>> package declaration to ArrayTest since my IDE (thanks Maurizio ;P) was
>> complaining about a duplicate class, since there was a test class with
>> the same name in the jextract test directory.
>> 
>> Related bug: https://bugs.openjdk.java.net/browse/JDK-8213343
>> Webrev: 
>> http://cr.openjdk.java.net/~jvernee/panama/webrevs/accessmode/webrev.00/
>> 
>> Please let me know what you think about this.
>> 
>> Thanks,
>> Jorn
>> 
>> [1] :
>> http://hg.openjdk.java.net/panama/dev/file/cfee9bc4ebe4/src/java.base/share/classes/jdk/internal/foreign/memory/BoundedMemoryRegion.java#l33


More information about the panama-dev mailing list