Skip to content

Added static helper methods for getting the underlying NativeLibrary instance from a Library interface instance or from a "registered" class.#1612

Closed
serinana47 wants to merge 1 commit intojava-native-access:masterfrom
serinana47:master
Closed

Conversation

@serinana47
Copy link
Copy Markdown

@serinana47 serinana47 commented Jun 25, 2024 •
edited
Loading

This adds static method getNativeLibrary() to the Library interface:

public interface Library {
    static NativeLibrary getNativeLibrary(final Library library) {
        return ((Handler)Proxy.getInvocationHandler(library)).getNativeLibrary();
    }
}

Useful, if the Library-based interface was loaded via Native.load() but we need to access the NativeLibrary instance.

One important use-case is the NativeLibrary.getGlobalVariableAddress() method!

I'm not aware of another way to get a global variable from the Library-based interface. If there is, please tell me 😏

(BTW: It is not trivial to figure out that getInvocationHandler() from java.lang.reflect.Proxy must be used to get the Handler, which then can can be used to get the NativeLibrary. This alone is enough reason to provide a dedicated method in the JNA interface for getting the NativeLibrary instance from a Library interface instance)

It will be used like this:

public interface SomeLibrary extends Library {
    SomeLibrary INSTANCE = Native.load("libfoo", SomeLibrary.class);

    static class NativeLibraryHolder {
        public static final NativeLibrary LIBRARY = Library.getNativeLibrary(INSTANCE); // <-- this is new !!!
    }

    static class SOME_GLOBAL_VARIABLE {
        private static final Pointer ADDRESS = NativeLibraryHolder.LIBRARY.getGlobalVariableAddress("FOOBAR");
        int get() { return ADDRESS.getInt(0L); }
        void set(final int newValue) { ADDRESS.setInt(0L, newValue); }
    }

    int some_function(int param);
}

@serinana47 serinana47 mentioned this pull request Jun 28, 2024
Closed
@matthiasblaesing
Copy link
Copy Markdown
Member

matthiasblaesing commented Jun 28, 2024

Makes sense to me. Another usecase is if you need to manually unload the library you need access to the NativeLibrary.

However you are only covering the "InvocationHandler" implementation, but JNA also supports a direct mapping, where there is no invocation handler, but adaption is done on the native level. See Native#register. From a quick glance, this should be doable if the method is moved to Native and thus gets access to registeredLibraries. The method then also needs a bit of documentation, when it can work (at least it should be set, that it is only intented to work with the objects returned by Native#register, Native#load or Native#loadLibrary (the latter could be omitted as its usage is deprecated in favor if load).

A test showing, that the code indeed works for both cases should be added to the unittests.

Please also ensure, that you add an entry to the CHANGES.md file to ensure people see the feature you implemented. Please check refer to the existing entries for the exact format. For the issue/PR reference you can choose freely whether you want to reference the issue you opened of this PR.

When you update the commit, please also ensure, that the author information is complete. There is an email, but no real name, please update that.

@serinana47 serinana47 force-pushed the master branch 2 times, most recently from 6a141f1 to 4137211 Compare July 2, 2024 10:12
@serinana47 serinana47 changed the title Added static helper method to Library interface for getting the wrapped NativeLibrary instance. Added static helper methods for getting the underlying NativeLibrary instance from a Library interface instance or from a "registered" class. Jul 2, 2024
@serinana47
Copy link
Copy Markdown
Author

serinana47 commented Jul 2, 2024

PR has been updated as desired (I hope so) 😏

@serinana47 serinana47 force-pushed the master branch 3 times, most recently from 6388436 to a5269a6 Compare July 2, 2024 16:10
@serinana47
Added static helper method to the Library interface for getting the w…
3bedcfb 
…rapped NativeLibrary instance + added static helper method to the Native class for getting the NativeLibrary instance that a "registered" class is bound to.
@matthiasblaesing
Copy link
Copy Markdown
Member

matthiasblaesing commented Jul 3, 2024

@serinana47 thank you for the update. I think we need another round. Please have a look here:

https://github.com/matthiasblaesing/jna/commits/pr-1612/

  • fea197a collects the two new methods together into Native. The namespace in Native is already polluted and the implementation must reside there for the the direct case. I also added a test checking behavior.
  • d8e469d brings the CHANGES.md entry in line with the others.

Feel free to integrate into your variant if you agree.

The commit still does not hold you real name. Please run git log -n 1 in your JNA checkout and have a look at the Author line. You see a username, not a real name. See here: https://www.git-tower.com/learn/git/faq/change-author-name-email

@serinana47
Copy link
Copy Markdown
Author

serinana47 commented Jul 3, 2024 •
edited
Loading

Thank you for your response.

  1. Why do we need an explicit null check? I agree that "fail fast" is a good idea. But I think that both functions would immediately fail with a NullPointerException anyway, if you try to call them with a null argument, so the explicit null check adds no value here. Also, TTBOMK, the Java convention is to throw NullPointerException for arguments that are null but shouldn't be null , and to throw IllegalArgumentException for other problems with the argument.

  2. I think the getNativeFunction() for instances of the Library interface should be in the Library.java file, because it is closely tied to the Library.Handler class, which also resides there. Furthermore, all existing code that deals with the Library.Handler class is in the Library.java file, so I think that is where the getNativeFunction() naturally belongs too.

  3. Sorry, I have no real name to share.

@matthiasblaesing
Copy link
Copy Markdown
Member

matthiasblaesing commented Jul 3, 2024

I'll finish this then myself. I won't merge changes without realnames. Sorry.

For the getNativeFile in Library: I don't take that argument. As a user you don't deal with Library.Handler. It is an implementation details, which might have better been concealed. On the other hand all users must use Native#load and thus this is the place where I would look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@serinana47 @matthiasblaesing