Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add MSI database read functions #1537

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

russellbanks
Copy link

This pull request adds database read functions to Msi.java:

I've added tests to MsiTest.java that verifies that they are mapped correctly.

Copy link
Member

@matthiasblaesing matthiasblaesing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. I made a first pass. Please see the inline comments. Please also see if you can come up with unittests, that only hit the error path. You also need to check your author information, as <74878137+russellbanks@users.noreply.github.com> is not a valid e-mail address.

* ERROR_INVALID_HANDLE - An invalid handle was passed to the function.
* ERROR_SUCCESS - The function succeeded.
*/
int MsiCloseHandle(Pointer hAny);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where does the definition of hAny come from? I see this in Msi.h:

typedef unsigned long MSIHANDLE;

If there is no other definition, it might be worth defining MSIHANDLE as a NativeMapped type and create MSIHandleByReference in the same as IntByReference is defined.

This would need to be updated for all call sites here.

Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add some tests that actually manipulate a test MSI? https://github.com/dblock/msiext has tons of examples ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants