-
-
Notifications
You must be signed in to change notification settings - Fork 223
Conversation
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.
Thanks for contributing. Yes, I think you can drop netstandard1.1 support as you suggest.
I have a few repo-contributing guideline conformance requests for you, and I'll be happy to merge.
src/IPHlpApi/IPHlpApi.cs
Outdated
bool sort, | ||
AddressFamily ipVersion, | ||
TcpTableType tcpTableType, | ||
int reserved); |
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 suggest uint
for reserved
here since we don't know what it'll be used for and the header file defines it as unsigned, and specifying 0
as an argument isn't any harder with it typed as uint
than it is if it were typed with int
.
src/IPHlpApi.Tests/IPHlpApiFacts.cs
Outdated
{ | ||
try | ||
{ | ||
tcpTable = Marshal.AllocHGlobal(tcpTableLength); |
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.
Tip: using pointers in C# instead of these IntPtr
types actually result in less code, and is often simpler to verify correctness.
/// <summary> | ||
/// The number of MIB_TCPROW_OWNER_PID elements in the table. | ||
/// </summary> | ||
public uint Length; |
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.
This should be called dwNumEntries
@AArnott Thanks for the feedback, I think I addressed all your remarks. |
@AArnott , apart from the other comments, I disabled #435 mentions dropping |
Feel free to make the ProjectReference in Win32 conditional on the TargetFramework to avoid the build break for now. Yes, #435 seems reasonable to do. I'd appreciate a PR for that. |
Adding
netstandard1.1
support is a bit problematic, is this really required?