[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