[sys] windows: add iphlpapi routing functions
Ian Chen (Gerrit)
Ian Chen has uploaded the change for review![]( "Open in Gerrit")
Commit message
windows: add iphlpapi routing functions
NotifyRouteChange2 registers to be notified for changes to IP route entries.
Call GetIpForwardEntry2 on received row to retrieve complete information.
GetIpForwardTable2 retrieves the full routing table.
FreeMibTable frees the buffer allocated by the functions that return tables of
network interfaces, addresses, and routes.
Change diff
diff --git a/windows/syscall_windows.go b/windows/syscall_windows.go
index 640f6b1..4640759 100644
--- a/windows/syscall_windows.go
+++ b/windows/syscall_windows.go
@@ -890,8 +890,12 @@
//sys MultiByteToWideChar(codePage uint32, dwFlags uint32, str *byte, nstr int32, wchar *uint16, nwchar int32) (nwrite int32, err error) = kernel32.MultiByteToWideChar
//sys getBestInterfaceEx(sockaddr unsafe.Pointer, pdwBestIfIndex *uint32) (errcode error) = iphlpapi.GetBestInterfaceEx
//sys GetIfEntry2Ex(level uint32, row *MibIfRow2) (errcode error) = iphlpapi.GetIfEntry2Ex
+//sys GetIpForwardEntry2(row *MibIpForwardRow2) (errcode error) = iphlpapi.GetIpForwardEntry2
+//sys GetIpForwardTable2(family uint16, table **MibIpForwardTable2) (errcode error) = iphlpapi.GetIpForwardTable2
//sys GetUnicastIpAddressEntry(row *MibUnicastIpAddressRow) (errcode error) = iphlpapi.GetUnicastIpAddressEntry
+//sys FreeMibTable(memory unsafe.Pointer) = iphlpapi.FreeMibTable
//sys NotifyIpInterfaceChange(family uint16, callback uintptr, callerContext unsafe.Pointer, initialNotification bool, notificationHandle *Handle) (errcode error) = iphlpapi.NotifyIpInterfaceChange
+//sys NotifyRouteChange2(family uint16, callback uintptr, callerContext unsafe.Pointer, initialNotification bool, notificationHandle *Handle) (errcode error) = iphlpapi.NotifyRouteChange2
//sys NotifyUnicastIpAddressChange(family uint16, callback uintptr, callerContext unsafe.Pointer, initialNotification bool, notificationHandle *Handle) (errcode error) = iphlpapi.NotifyUnicastIpAddressChange
//sys CancelMibChangeNotify2(notificationHandle Handle) (errcode error) = iphlpapi.CancelMibChangeNotify2
diff --git a/windows/types_windows.go b/windows/types_windows.go
index 958bcf4..f49c0aa 100644
--- a/windows/types_windows.go
+++ b/windows/types_windows.go
@@ -2298,6 +2298,82 @@
OutQLen uint64
}
+// IP_ADDRESS_PREFIX stores an IP address prefix. See
+// https://learn.microsoft.com/en-us/windows/win32/api/netioapi/ns-netioapi-ip_address_prefix.
+type IpAddressPrefix struct {
+ Prefix RawSockaddrInet6 // SOCKADDR_INET union
+ PrefixLength uint8
+}
+
+// NL_ROUTE_ORIGIN enumeration from nldef.h or
+// https://learn.microsoft.com/en-us/windows/win32/api/nldef/ne-nldef-nl_route_origin.
+const (
+ NlroManual = 0
+ NlroWellKnown = 1
+ NlroDHCP = 2
+ NlroRouterAdvertisement = 3
+ Nlro6to4 = 4
+)
+
+// NL_ROUTE_ORIGIN enumeration from nldef.h or
+// https://learn.microsoft.com/en-us/windows/win32/api/nldef/ne-nldef-nl_route_protocol.
+const (
+ MIB_IPPROTO_OTHER = 1
+ MIB_IPPROTO_LOCAL = 2
+ MIB_IPPROTO_NETMGMT = 3
+ MIB_IPPROTO_ICMP = 4
+ MIB_IPPROTO_EGP = 5
+ MIB_IPPROTO_GGP = 6
+ MIB_IPPROTO_HELLO = 7
+ MIB_IPPROTO_RIP = 8
+ MIB_IPPROTO_IS_IS = 9
+ MIB_IPPROTO_ES_IS = 10
+ MIB_IPPROTO_CISCO = 11
+ MIB_IPPROTO_BBN = 12
+ MIB_IPPROTO_OSPF = 13
+ MIB_IPPROTO_BGP = 14
+ MIB_IPPROTO_IDPR = 15
+ MIB_IPPROTO_EIGRP = 16
+ MIB_IPPROTO_DVMRP = 17
+ MIB_IPPROTO_RPL = 18
+ MIB_IPPROTO_DHCP = 19
+ MIB_IPPROTO_NT_AUTOSTATIC = 10002
+ MIB_IPPROTO_NT_STATIC = 10006
+ MIB_IPPROTO_NT_STATIC_NON_DOD = 10007
+)
+
+// MIB_IPFORWARD_ROW2 stores information about an IP route entry. See
+// https://learn.microsoft.com/en-us/windows/win32/api/netioapi/ns-netioapi-mib_ipforward_row2.
+type MibIpForwardRow2 struct {
+ InterfaceLuid uint64
+ InterfaceIndex uint32
+ DestinationPrefix IpAddressPrefix
+ NextHop RawSockaddrInet6 // SOCKADDR_INET union
+ SitePrefixLength uint8
+ ValidLifetime uint32
+ PreferredLifetime uint32
+ Metric uint32
+ Protocol uint32
+ Loopback uint8
+ AutoconfigureAddress uint8
+ Publish uint8
+ Immortal uint8
+ Age uint32
+ Origin uint32
+}
+
+// MIB_IPFORWARD_TABLE2 contains a table of IP route entries. See
+// https://learn.microsoft.com/en-us/windows/win32/api/netioapi/ns-netioapi-mib_ipforward_table2.
+type MibIpForwardTable2 struct {
+ NumEntries uint32
+ Table [1]MibIpForwardRow2
+}
+
+// Rows returns the IP route entries in the table.
+func (t *MibIpForwardTable2) Rows() []MibIpForwardRow2 {
+ return unsafe.Slice(&t.Table[0], t.NumEntries)
+}
+
// MIB_UNICASTIPADDRESS_ROW stores information about a unicast IP address. See
// https://learn.microsoft.com/en-us/windows/win32/api/netioapi/ns-netioapi-mib_unicastipaddress_row.
type MibUnicastIpAddressRow struct {
diff --git a/windows/zsyscall_windows.go b/windows/zsyscall_windows.go
index a58bc48..b8f14f3 100644
--- a/windows/zsyscall_windows.go
+++ b/windows/zsyscall_windows.go
@@ -182,13 +182,17 @@
procDwmGetWindowAttribute = moddwmapi.NewProc("DwmGetWindowAttribute")
procDwmSetWindowAttribute = moddwmapi.NewProc("DwmSetWindowAttribute")
procCancelMibChangeNotify2 = modiphlpapi.NewProc("CancelMibChangeNotify2")
+ procFreeMibTable = modiphlpapi.NewProc("FreeMibTable")
procGetAdaptersAddresses = modiphlpapi.NewProc("GetAdaptersAddresses")
procGetAdaptersInfo = modiphlpapi.NewProc("GetAdaptersInfo")
procGetBestInterfaceEx = modiphlpapi.NewProc("GetBestInterfaceEx")
procGetIfEntry = modiphlpapi.NewProc("GetIfEntry")
procGetIfEntry2Ex = modiphlpapi.NewProc("GetIfEntry2Ex")
+ procGetIpForwardEntry2 = modiphlpapi.NewProc("GetIpForwardEntry2")
+ procGetIpForwardTable2 = modiphlpapi.NewProc("GetIpForwardTable2")
procGetUnicastIpAddressEntry = modiphlpapi.NewProc("GetUnicastIpAddressEntry")
procNotifyIpInterfaceChange = modiphlpapi.NewProc("NotifyIpInterfaceChange")
+ procNotifyRouteChange2 = modiphlpapi.NewProc("NotifyRouteChange2")
procNotifyUnicastIpAddressChange = modiphlpapi.NewProc("NotifyUnicastIpAddressChange")
procAddDllDirectory = modkernel32.NewProc("AddDllDirectory")
procAssignProcessToJobObject = modkernel32.NewProc("AssignProcessToJobObject")
@@ -1622,6 +1626,11 @@
return
}
+func FreeMibTable(memory unsafe.Pointer) {
+ syscall.Syscall(procFreeMibTable.Addr(), 1, uintptr(memory), 0, 0)
+ return
+}
+
func GetAdaptersAddresses(family uint32, flags uint32, reserved uintptr, adapterAddresses *IpAdapterAddresses, sizePointer *uint32) (errcode error) {
r0, _, _ := syscall.Syscall6(procGetAdaptersAddresses.Addr(), 5, uintptr(family), uintptr(flags), uintptr(reserved), uintptr(unsafe.Pointer(adapterAddresses)), uintptr(unsafe.Pointer(sizePointer)), 0)
if r0 != 0 {
@@ -1662,6 +1671,22 @@
return
}
+func GetIpForwardEntry2(row *MibIpForwardRow2) (errcode error) {
+ r0, _, _ := syscall.Syscall(procGetIpForwardEntry2.Addr(), 1, uintptr(unsafe.Pointer(row)), 0, 0)
+ if r0 != 0 {
+ errcode = syscall.Errno(r0)
+ }
+ return
+}
+
+func GetIpForwardTable2(family uint16, table **MibIpForwardTable2) (errcode error) {
+ r0, _, _ := syscall.Syscall(procGetIpForwardTable2.Addr(), 2, uintptr(family), uintptr(unsafe.Pointer(table)), 0)
+ if r0 != 0 {
+ errcode = syscall.Errno(r0)
+ }
+ return
+}
+
func GetUnicastIpAddressEntry(row *MibUnicastIpAddressRow) (errcode error) {
r0, _, _ := syscall.Syscall(procGetUnicastIpAddressEntry.Addr(), 1, uintptr(unsafe.Pointer(row)), 0, 0)
if r0 != 0 {
@@ -1682,6 +1707,18 @@
return
}
+func NotifyRouteChange2(family uint16, callback uintptr, callerContext unsafe.Pointer, initialNotification bool, notificationHandle *Handle) (errcode error) {
+ var _p0 uint32
+ if initialNotification {
+ _p0 = 1
+ }
+ r0, _, _ := syscall.Syscall6(procNotifyRouteChange2.Addr(), 5, uintptr(family), uintptr(callback), uintptr(callerContext), uintptr(_p0), uintptr(unsafe.Pointer(notificationHandle)), 0)
+ if r0 != 0 {
+ errcode = syscall.Errno(r0)
+ }
+ return
+}
+
func NotifyUnicastIpAddressChange(family uint16, callback uintptr, callerContext unsafe.Pointer, initialNotification bool, notificationHandle *Handle) (errcode error) {
var _p0 uint32
if initialNotification {
Change information
- M windows/syscall_windows.go
- M windows/types_windows.go
- M windows/zsyscall_windows.go
Related details
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Chen (Gerrit)
Ian Chen uploaded new patchset![]( "Open in Gerrit")
Related details
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Chen (Gerrit)
Ian Chen uploaded new patchset![]( "Open in Gerrit")
Related details
- Alex Brainman
- Brad Fitzpatrick
- Quim Muntal
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Chen (Gerrit)
Ian Chen added 1 comment![]( "Open in Gerrit")
Regenerated after CL 691715 (reinstating SyscallN).
Related details
- Alex Brainman
- Brad Fitzpatrick
- Quim Muntal
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Sean Liao (Gerrit)
Sean Liao voted Commit-Queue+1![]( "Open in Gerrit")
- Alex Brainman
- Brad Fitzpatrick
- Ian Chen
- Quim Muntal
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Quim Muntal (Gerrit)
Quim Muntal added 2 comments![]( "Open in Gerrit")
Prefix RawSockaddrInet6 // SOCKADDR_INET unionI don't like representing `SOCKADDR_INET` with a `RawSockaddrInet6`, as the former is a union that can't always be safely casted into the latter. Better do something like this: https://github.com/netbirdio/netbird/blob/71e944fa57868ce38a30bdd7267de5dbd52a72cb/client/internal/routemanager/systemops/systemops_windows.go#L86-L94.
And then add a helper to access access the address in a safe manner.
NextHop RawSockaddrInet6 // SOCKADDR_INET unionSame as previous comment.
Related details
- Alex Brainman
- Brad Fitzpatrick
- Ian Chen
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Chen (Gerrit)
Ian Chen added 1 comment![]( "Open in Gerrit")
Prefix RawSockaddrInet6 // SOCKADDR_INET unionI don't like representing `SOCKADDR_INET` with a `RawSockaddrInet6`, as the former is a union that can't always be safely casted into the latter. Better do something like this: https://github.com/netbirdio/netbird/blob/71e944fa57868ce38a30bdd7267de5dbd52a72cb/client/internal/routemanager/systemops/systemops_windows.go#L86-L94.
And then add a helper to access access the address in a safe manner.
I don't like representing `SOCKADDR_INET` with a `RawSockaddrInet6`, as the former is a union that can't always be safely casted into the latter. Better do something like this: https://github.com/netbirdio/netbird/blob/71e944fa57868ce38a30bdd7267de5dbd52a72cb/client/internal/routemanager/systemops/systemops_windows.go#L86-L94.
Your linked example is wrong. The actual `SOCKADDR_INET` union requires 4-byte alignment, because there are `uint32` fields in `SOCKADDR_IN6`.
NetBird's definition only requires 2-byte alignment. It just happens to work here, because the field sits right after `InterfaceIndex uint32`.
BTW, we already have `MibUnicastIpAddressRow` where the `Address` field is a `SOCKADDR_INET` union and is represented by a `RawSockaddrInet6`.
And then add a helper to access access the address in a safe manner.
I'm not sure how useful such helper methods are. User code will simply switch on `Family` and convert to the right `RawSockaddr*` type with `unsafe`.
Related details
- Alex Brainman
- Brad Fitzpatrick
- Quim Muntal
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Quim Muntal (Gerrit)
Quim Muntal added 1 comment![]( "Open in Gerrit")
Prefix RawSockaddrInet6 // SOCKADDR_INET unionIan ChenI don't like representing `SOCKADDR_INET` with a `RawSockaddrInet6`, as the former is a union that can't always be safely casted into the latter. Better do something like this: https://github.com/netbirdio/netbird/blob/71e944fa57868ce38a30bdd7267de5dbd52a72cb/client/internal/routemanager/systemops/systemops_windows.go#L86-L94.
And then add a helper to access access the address in a safe manner.
I don't like representing `SOCKADDR_INET` with a `RawSockaddrInet6`, as the former is a union that can't always be safely casted into the latter. Better do something like this: https://github.com/netbirdio/netbird/blob/71e944fa57868ce38a30bdd7267de5dbd52a72cb/client/internal/routemanager/systemops/systemops_windows.go#L86-L94.
Your linked example is wrong. The actual `SOCKADDR_INET` union requires 4-byte alignment, because there are `uint32` fields in `SOCKADDR_IN6`.
NetBird's definition only requires 2-byte alignment. It just happens to work here, because the field sits right after `InterfaceIndex uint32`.
BTW, we already have `MibUnicastIpAddressRow` where the `Address` field is a `SOCKADDR_INET` union and is represented by a `RawSockaddrInet6`.
And then add a helper to access access the address in a safe manner.
I'm not sure how useful such helper methods are. User code will simply switch on `Family` and convert to the right `RawSockaddr*` type with `unsafe`.
Your linked example is wrong. The actual SOCKADDR_INET union requires 4-byte alignment, because there are uint32 fields in SOCKADDR_IN6.
Didn't check the actual memory layout, but you got the idea 😊.
BTW, we already have MibUnicastIpAddressRow where the Address field is a SOCKADDR_INET union and is represented by a RawSockaddrInet6.
Yep, that's unfortunate, I prefer to do it better this time.
I'm not sure how useful such helper methods are. User code will simply switch on Family and convert to the right RawSockaddr* type with unsafe.
I'm fine leaving that method out of this CL.
Related details
- Alex Brainman
- Brad Fitzpatrick
- Ian Chen
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Chen (Gerrit)
Ian Chen added 1 comment![]( "Open in Gerrit")
Prefix RawSockaddrInet6 // SOCKADDR_INET unionIan ChenI don't like representing `SOCKADDR_INET` with a `RawSockaddrInet6`, as the former is a union that can't always be safely casted into the latter. Better do something like this: https://github.com/netbirdio/netbird/blob/71e944fa57868ce38a30bdd7267de5dbd52a72cb/client/internal/routemanager/systemops/systemops_windows.go#L86-L94.
And then add a helper to access access the address in a safe manner.
I don't like representing `SOCKADDR_INET` with a `RawSockaddrInet6`, as the former is a union that can't always be safely casted into the latter. Better do something like this: https://github.com/netbirdio/netbird/blob/71e944fa57868ce38a30bdd7267de5dbd52a72cb/client/internal/routemanager/systemops/systemops_windows.go#L86-L94.
Your linked example is wrong. The actual `SOCKADDR_INET` union requires 4-byte alignment, because there are `uint32` fields in `SOCKADDR_IN6`.
NetBird's definition only requires 2-byte alignment. It just happens to work here, because the field sits right after `InterfaceIndex uint32`.
BTW, we already have `MibUnicastIpAddressRow` where the `Address` field is a `SOCKADDR_INET` union and is represented by a `RawSockaddrInet6`.
And then add a helper to access access the address in a safe manner.
I'm not sure how useful such helper methods are. User code will simply switch on `Family` and convert to the right `RawSockaddr*` type with `unsafe`.
User code will simply switch on `Family` and convert to the right `RawSockaddr*` type with `unsafe`.
An example of how this might look like: https://github.com/database64128/ddns-go/blob/4c2cc44e2ca74a9dc99cf50a704b4db4ccda9b51/producer/win32iphlp/win32iphlp_windows.go#L384-L399
As you can see, the conversion is quite trivial. Helper methods would not be useful here.
Related details
- Alex Brainman
- Ian Chen
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Chen (Gerrit)
Ian Chen uploaded new patchset![]( "Open in Gerrit")
- TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
Related details
- Alex Brainman
- Ian Chen
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Chen (Gerrit)
Ian Chen uploaded new patchset![]( "Open in Gerrit")
Related details
- Alex Brainman
- Ian Chen
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Chen (Gerrit)
Ian Chen added 2 comments![]( "Open in Gerrit")
Ian ChenI don't like representing `SOCKADDR_INET` with a `RawSockaddrInet6`, as the former is a union that can't always be safely casted into the latter. Better do something like this: https://github.com/netbirdio/netbird/blob/71e944fa57868ce38a30bdd7267de5dbd52a72cb/client/internal/routemanager/systemops/systemops_windows.go#L86-L94.
And then add a helper to access access the address in a safe manner.
Ian ChenI don't like representing `SOCKADDR_INET` with a `RawSockaddrInet6`, as the former is a union that can't always be safely casted into the latter. Better do something like this: https://github.com/netbirdio/netbird/blob/71e944fa57868ce38a30bdd7267de5dbd52a72cb/client/internal/routemanager/systemops/systemops_windows.go#L86-L94.
Your linked example is wrong. The actual `SOCKADDR_INET` union requires 4-byte alignment, because there are `uint32` fields in `SOCKADDR_IN6`.
NetBird's definition only requires 2-byte alignment. It just happens to work here, because the field sits right after `InterfaceIndex uint32`.
BTW, we already have `MibUnicastIpAddressRow` where the `Address` field is a `SOCKADDR_INET` union and is represented by a `RawSockaddrInet6`.
And then add a helper to access access the address in a safe manner.
I'm not sure how useful such helper methods are. User code will simply switch on `Family` and convert to the right `RawSockaddr*` type with `unsafe`.
User code will simply switch on `Family` and convert to the right `RawSockaddr*` type with `unsafe`.
An example of how this might look like: https://github.com/database64128/ddns-go/blob/4c2cc44e2ca74a9dc99cf50a704b4db4ccda9b51/producer/win32iphlp/win32iphlp_windows.go#L384-L399
As you can see, the conversion is quite trivial. Helper methods would not be useful here.
Added `SockaddrInet` with `Data [6]uint32` for the correct alignment requirement. Please take another look, thanks. 😊
NextHop RawSockaddrInet6 // SOCKADDR_INET unionIan ChenSame as previous comment.
Done
Related details
- Alex Brainman
- Quim Muntal
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Chen (Gerrit)
Ian Chen added 1 comment![]( "Open in Gerrit")
Prefix RawSockaddrInet6 // SOCKADDR_INET unionIan ChenI don't like representing `SOCKADDR_INET` with a `RawSockaddrInet6`, as the former is a union that can't always be safely casted into the latter. Better do something like this: https://github.com/netbirdio/netbird/blob/71e944fa57868ce38a30bdd7267de5dbd52a72cb/client/internal/routemanager/systemops/systemops_windows.go#L86-L94.
And then add a helper to access access the address in a safe manner.
Ian ChenI don't like representing `SOCKADDR_INET` with a `RawSockaddrInet6`, as the former is a union that can't always be safely casted into the latter. Better do something like this: https://github.com/netbirdio/netbird/blob/71e944fa57868ce38a30bdd7267de5dbd52a72cb/client/internal/routemanager/systemops/systemops_windows.go#L86-L94.
Your linked example is wrong. The actual `SOCKADDR_INET` union requires 4-byte alignment, because there are `uint32` fields in `SOCKADDR_IN6`.
NetBird's definition only requires 2-byte alignment. It just happens to work here, because the field sits right after `InterfaceIndex uint32`.
BTW, we already have `MibUnicastIpAddressRow` where the `Address` field is a `SOCKADDR_INET` union and is represented by a `RawSockaddrInet6`.
And then add a helper to access access the address in a safe manner.
I'm not sure how useful such helper methods are. User code will simply switch on `Family` and convert to the right `RawSockaddr*` type with `unsafe`.
Ian ChenUser code will simply switch on `Family` and convert to the right `RawSockaddr*` type with `unsafe`.
An example of how this might look like: https://github.com/database64128/ddns-go/blob/4c2cc44e2ca74a9dc99cf50a704b4db4ccda9b51/producer/win32iphlp/win32iphlp_windows.go#L384-L399
As you can see, the conversion is quite trivial. Helper methods would not be useful here.
Added `SockaddrInet` with `Data [6]uint32` for the correct alignment requirement. Please take another look, thanks. 😊
Yep, that's unfortunate, I prefer to do it better this time.
BTW I also updated that field to use the new type. `MibUnicastIpAddressRow` was added by me a few months ago, and it seems like I'm still the only user.
Let me know whether this is an acceptable breaking change.
Quim Muntal (Gerrit)
Quim Muntal voted and added 1 comment![]( "Open in Gerrit")
Votes added by Quim Muntal
| Code-Review | +2 |
| Commit-Queue | +1 |
1 comment
Related details
- Alex Brainman
- Ian Chen
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Pratt (Gerrit)
Michael Pratt added 2 comments![]( "Open in Gerrit")
type SockaddrInet struct {Should this be called RawSockaddrInet? It is more like RawSockaddrInet4 and RawSockaddrInet6 than SockaddrInet4 and SockaddrInet6.
How do we expect users to use this type? Should there be helpers for converting to/from SockaddrInet4 and SockaddrInet6?
Address SockaddrInetChanging the type of a field violates the Go 1 Compatibility promise. (Though we are sometimes more lenient on that in `syscall`).
It does seem like the old type was a mistake, but I'm not sure this can be safely changed.
Related details
- Alex Brainman
- Ian Chen
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Chen (Gerrit)
Ian Chen uploaded new patchset![]( "Open in Gerrit")
- Code-Review: +2 by Quim Muntal
- TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
- Alex Brainman
- Ian Chen
- Quim Muntal
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Chen (Gerrit)
Ian Chen added 2 comments![]( "Open in Gerrit")
Should this be called RawSockaddrInet? It is more like RawSockaddrInet4 and RawSockaddrInet6 than SockaddrInet4 and SockaddrInet6.
How do we expect users to use this type? Should there be helpers for converting to/from SockaddrInet4 and SockaddrInet6?
Should this be called RawSockaddrInet? It is more like RawSockaddrInet4 and RawSockaddrInet6 than SockaddrInet4 and SockaddrInet6.
Renamed and moved to the file where `RawSockaddrInet{4,6}` are defined.
How do we expect users to use this type? Should there be helpers for converting to/from SockaddrInet4 and SockaddrInet6?
Users are expected to switch on `Family` and use `unsafe` to convert to `*RawSockaddrInet{4,6}`. IMO helpers are unlikely to provide value to users. Earlier discussions also mentioned this.
Changing the type of a field violates the Go 1 Compatibility promise. (Though we are sometimes more lenient on that in `syscall`).
It does seem like the old type was a mistake, but I'm not sure this can be safely changed.
- Alex Brainman
- Michael Pratt
- Quim Muntal
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Pratt (Gerrit)
Michael Pratt voted and added 3 comments![]( "Open in Gerrit")
Votes added by Michael Pratt
| Code-Review | +1 |
3 comments
type SockaddrInet struct {Ian ChenShould this be called RawSockaddrInet? It is more like RawSockaddrInet4 and RawSockaddrInet6 than SockaddrInet4 and SockaddrInet6.
How do we expect users to use this type? Should there be helpers for converting to/from SockaddrInet4 and SockaddrInet6?
Should this be called RawSockaddrInet? It is more like RawSockaddrInet4 and RawSockaddrInet6 than SockaddrInet4 and SockaddrInet6.
Renamed and moved to the file where `RawSockaddrInet{4,6}` are defined.
How do we expect users to use this type? Should there be helpers for converting to/from SockaddrInet4 and SockaddrInet6?
Users are expected to switch on `Family` and use `unsafe` to convert to `*RawSockaddrInet{4,6}`. IMO helpers are unlikely to provide value to users. Earlier discussions also mentioned this.
OK. I think a comment on `RawSockaddrInet` saying it can be converted to/from the other types would help users discover this.
Address SockaddrInetIan ChenChanging the type of a field violates the Go 1 Compatibility promise. (Though we are sometimes more lenient on that in `syscall`).
It does seem like the old type was a mistake, but I'm not sure this can be safely changed.
Reverted the change.
Thanks.
I can't find a single use of `MibUnicastIpAddressRow` on GitHub, so I think there is an argument that we can change this as a bug in the API. But that should have a short proposal, so not part of the this CL.
Related details
- Alex Brainman
- Ian Chen
- Quim Muntal
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Pratt (Gerrit)
Michael Pratt voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Quim Muntal (Gerrit)
Quim Muntal voted Code-Review+2![]( "Open in Gerrit")
| Code-Review | +2 |
Related details
- Alex Brainman
- Ian Chen
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Quim Muntal (Gerrit)
Quim Muntal added 1 comment![]( "Open in Gerrit")
// SOCKADDR_INET is a union that contains an IPv4, an IPv6 address, or an address family. See```suggestion
// RawSockaddrInet is a union that contains an IPv4, an IPv6 address, or an address family. See
```
Related details
- Alex Brainman
- Ian Chen
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Chen (Gerrit)
Ian Chen uploaded new patchset![]( "Open in Gerrit")
- Code-Review: +2 by Quim Muntal, +1 by Michael Pratt
- TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
- Alex Brainman
- Ian Chen
- Michael Pratt
- Quim Muntal
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Chen (Gerrit)
Ian Chen added 2 comments![]( "Open in Gerrit")
// SOCKADDR_INET is a union that contains an IPv4, an IPv6 address, or an address family. See```suggestion
// RawSockaddrInet is a union that contains an IPv4, an IPv6 address, or an address family. See
```
Done.
type SockaddrInet struct {Ian ChenShould this be called RawSockaddrInet? It is more like RawSockaddrInet4 and RawSockaddrInet6 than SockaddrInet4 and SockaddrInet6.
How do we expect users to use this type? Should there be helpers for converting to/from SockaddrInet4 and SockaddrInet6?
Michael PrattShould this be called RawSockaddrInet? It is more like RawSockaddrInet4 and RawSockaddrInet6 than SockaddrInet4 and SockaddrInet6.
Renamed and moved to the file where `RawSockaddrInet{4,6}` are defined.
How do we expect users to use this type? Should there be helpers for converting to/from SockaddrInet4 and SockaddrInet6?
Users are expected to switch on `Family` and use `unsafe` to convert to `*RawSockaddrInet{4,6}`. IMO helpers are unlikely to provide value to users. Earlier discussions also mentioned this.
OK. I think a comment on `RawSockaddrInet` saying it can be converted to/from the other types would help users discover this.
- Alex Brainman
- Michael Pratt
- Quim Muntal
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Chen (Gerrit)
Ian Chen added 1 comment![]( "Open in Gerrit")
Ian Chen (Gerrit)
Ian Chen added 1 comment![]( "Open in Gerrit")
@quimm...@gmail.com Can you please +2 this change again? I'm not a trusted user, so your previous +2 vote was removed after your suggested change. Thanks.