Skip to content

Add GetIfEntry, GetNetworkParams, and supporting IPHlpAPI structures#983

Closed
dbwiddis wants to merge 2 commits into
java-native-access:masterfrom
dbwiddis:iphlpapi
Closed

Add GetIfEntry, GetNetworkParams, and supporting IPHlpAPI structures#983
dbwiddis wants to merge 2 commits into
java-native-access:masterfrom
dbwiddis:iphlpapi

Conversation

@dbwiddis

@dbwiddis dbwiddis commented Jul 1, 2018

Copy link
Copy Markdown
Contributor

No description provided.

@dbwiddis
dbwiddis force-pushed the iphlpapi branch 4 times, most recently from 6d925b3 to 482b0d3 Compare July 2, 2018 15:53
"InUcastOctets", "InMulticastOctets", "InBroadcastOctets", "OutOctets", "OutUcastPkts", "OutNUcastPkts",
"OutDiscards", "OutErrors", "OutUcastOctets", "OutMulticastOctets", "OutBroadcastOctets", "OutQLen" })
class MIB_IF_ROW2 extends Structure {
public long InterfaceLuid; // 64-bit union

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this safe to use the raw value here instead of the union?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's safe, as it's 64-bits. The alternative would be a multi-layer NET_LUID union in JNA with one alternative being a 64-bit Value (the intended return) and the actual information encoded in 5 of the 8 bytes. See https://msdn.microsoft.com/library/windows/hardware/ff568747 . Happy to try to add that NET_LUID structure if I can figure out how; but at this layer it's possible for it to be returned and then parsed by the user with shifting and bitmasks.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I agree, retrieving the correct value can be implemented as a helper method on the structure, if needed.

"OutDiscards", "OutErrors", "OutUcastOctets", "OutMulticastOctets", "OutBroadcastOctets", "OutQLen" })
class MIB_IF_ROW2 extends Structure {
public long InterfaceLuid; // 64-bit union
public ULONG InterfaceIndex;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would int be an alternative? (applies to other definitions as well).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Also applies to some of my other PRs. As a structure mapping it would work, however, I have tried (not always consistently) to use the mapped type when variables are unsigned. I've gone back and forth a few times on many of these... using int, we require the user to know to interpret the unsigned value, while ULONG or DWORD offer longvalue() options.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Your argument is sound - the question was really meant as a question, so if you think it would be better to bind it as a wrapped value, I'm ok with it too. The benefit I see in the int is, that it is the "natural" integral number type in java and thus easier to deal with.
If you want to switch back to DWORD, feel free to do so - please indicate your preference, I'll hold of merging till then.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's ok to leave it as int here, because the javadoc makes it clear what the underlying value is and there's likely a tiny performance improvement to avoiding the extra object creation. As I said I went back and forth on this (I literally did search-and-replace, and then undo, at least twice) before submitting.

I might suggest a Global JNA util method that converts unsigned values, though, so users don't have to always reinvent the wheel.

*/
@FieldOrder({ "Next", "IpAddress", "IpMask", "Context" })
class IP_ADDR_STRING extends Structure {
public ByReference Next;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please qualify this to IP_ADDR_STRING.ByReference I had to look multiple times.

@dbwiddis

dbwiddis commented Jul 3, 2018

Copy link
Copy Markdown
Contributor Author

Made the changes requested except left the NET_LUID union as a long. I'm open to a byte[8] instead if it'd be more clear. I don't think there's much value in coding up the NET_LUID union itself.

@matthiasblaesing

matthiasblaesing commented Jul 4, 2018

Copy link
Copy Markdown
Member

I merged the three changesets via:

2287b92

Thank you for your contribution.

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