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

WMI Query enhancement #1084

Merged
merged 2 commits into from
Apr 15, 2019
Merged

Conversation

matthiasblaesing
Copy link
Member

No description provided.

@matthiasblaesing
Copy link
Member Author

@dbwiddis could you please have a look at this changeset?

if(((vtType & VT_ARRAY) == VT_ARRAY) ||
((vtType & VT_UNKNOWN) == VT_UNKNOWN)||
((vtType & VT_DISPATCH) == VT_DISPATCH)) {
values.add(vtType, cimType, property, null);
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't the VT_VECTOR and VT_BYREF masks require the same treatment as VT_ARRAY? Or are they simply not valid return types from CIM/WMI, so don't need to be handled?

Copy link
Member Author

Choose a reason for hiding this comment

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

I have a strong WTF moment. Till now I did not realise, that there is a VT_VECTOR. I added that to the exclusion list. VT_BYREF is a different matter, as it only differs in the way the value is passed.

@@ -282,4 +291,22 @@ public void testWmiOperatingSystem() {
assertEquals(Variant.VT_BOOL, os.getVtType(OperatingSystemProperty.PRIMARY));
assertNotNull(os.getValue(OperatingSystemProperty.PRIMARY, 0));
}

enum Win32_DiskDrive_Values {
Caption,
Copy link
Contributor

Choose a reason for hiding this comment

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

Some coding convention analysis tools will complain that these (constant) enum values aren't all uppercase. I think it also may make the later test assertions more readable. It still works as WMI queries are case-insensitive.

Copy link
Member Author

Choose a reason for hiding this comment

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

I tend to disagree - the casing matches the version returned by GetNames - anyway adjusted.

Copy link
Contributor

Choose a reason for hiding this comment

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

The earlier enums in the test class (that I wrote) are in all caps. :) But I would agree with you that JNA tends to try to keep consistent with Windows cases so I'm ok with this.

@@ -341,7 +346,13 @@ public void setWmiClassName(String wmiClassName) {
break;
// Unimplemented type. User must cast
default:
values.add(vtType, cimType, property, pVal.getValue());
if(((vtType & VT_ARRAY) == VT_ARRAY) ||
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to handle (as a case before the default) VT_EMPTY unless we're sure it will return VT_ARRAY|VT_EMPTY. Might also want to throw in VT_NULL (return null).

Copy link
Member Author

Choose a reason for hiding this comment

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

VT_NULL was already handled, I added VT_EMPTY to be handled the same way

return GetNames(wszQualifierName == null ? null : new WString(wszQualifierName), lFlags, pQualifierVal, pNames);
}

public HRESULT GetNames(WString wszQualifierName, int lFlags, VARIANT.ByReference pQualifierVal, PointerByReference pNames) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I like this addition.

@matthiasblaesing
Copy link
Member Author

Thank you for the review - I updated the PR.

@dbwiddis
Copy link
Contributor

Looks good to me!

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.

2 participants