-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Add GetIfEntry, GetNetworkParams, and supporting IPHlpAPI structures #983
Conversation
6d925b3
to
482b0d3
Compare
"InUcastOctets", "InMulticastOctets", "InBroadcastOctets", "OutOctets", "OutUcastPkts", "OutNUcastPkts", | ||
"OutDiscards", "OutErrors", "OutUcastOctets", "OutMulticastOctets", "OutBroadcastOctets", "OutQLen" }) | ||
class MIB_IF_ROW2 extends Structure { | ||
public long InterfaceLuid; // 64-bit union |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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.
Made the changes requested except left the |
I merged the three changesets via: Thank you for your contribution. |
No description provided.