Add NetworkAdaptor to dart:io.

Add NetworkAdaptor to dart:io.

NetworkAdaptor contains a static method, list, that will query the
underlaying system for active network adaptors.

BUG=
R=sgjesse@google.com

Committed: https://code.google.com/p/dart/source/detail?r=24267

[Anders Johnsen, 2013-06-10 10:25:10]

Linux impl for now, please comment on API before I approach other platforms.

[Søren Gjesse, 2013-06-10 12:26:38]

https://codereview.chromium.org/16514010/diff/2001/runtime/bin/socket.h
File runtime/bin/socket.h (right):

https://codereview.chromium.org/16514010/diff/2001/runtime/bin/socket.h#newco...
runtime/bin/socket.h:101: class AdaptorSocketAddress : public SocketAddress {
Instead of inheriting here why not have the SocketAddress as a member?

https://codereview.chromium.org/16514010/diff/2001/runtime/bin/socket.h#newco...
runtime/bin/socket.h:104: : SocketAddress(sockaddr), name_(name) {}
4 space indent.

https://codereview.chromium.org/16514010/diff/2001/sdk/lib/io/socket.dart
File sdk/lib/io/socket.dart (right):

https://codereview.chromium.org/16514010/diff/2001/sdk/lib/io/socket.dart#new...
sdk/lib/io/socket.dart:102: class NetworkAdaptor {
We should call this NetworkInterface.

https://codereview.chromium.org/16514010/diff/2001/sdk/lib/io/socket.dart#new...
sdk/lib/io/socket.dart:109: InternetAddressType type: InternetAddressType.ANY});
I think we should add a flag here to filter out link local addresses as well. It
should probably filter out by default

  ...
  bool includeLinkLocal: false});
  ...

With IPv6 the link local address is always present even if there is a routeable
address. For IPv4 they normally go away after an routable address has been
assigned.

For IPv6 it is fe80::/10 and for IPv4 it is 169.254.1.0 to 169.254.254.255. See
http://en.wikipedia.org/wiki/Reserved_IP_addresses.

Alternatively (and maybe better) add an isLinkLocal to the InternetAddress and
make sure that these addresses are always last in the addresses list.

[Anders Johnsen, 2013-06-20 13:42:08]

PTAL, and I'll start on other platforms.

https://codereview.chromium.org/16514010/diff/2001/sdk/lib/io/socket.dart
File sdk/lib/io/socket.dart (right):

https://codereview.chromium.org/16514010/diff/2001/sdk/lib/io/socket.dart#new...
sdk/lib/io/socket.dart:102: class NetworkAdaptor {
On 2013/06/10 12:26:38, Søren Gjesse wrote:
> We should call this NetworkInterface.

Done.

https://codereview.chromium.org/16514010/diff/2001/sdk/lib/io/socket.dart#new...
sdk/lib/io/socket.dart:109: InternetAddressType type: InternetAddressType.ANY});
On 2013/06/10 12:26:38, Søren Gjesse wrote:
> I think we should add a flag here to filter out link local addresses as well.
It
> should probably filter out by default
> 
>   ...
>   bool includeLinkLocal: false});
>   ...
> 
> With IPv6 the link local address is always present even if there is a
routeable
> address. For IPv4 they normally go away after an routable address has been
> assigned.
> 
> For IPv6 it is fe80::/10 and for IPv4 it is 169.254.1.0 to 169.254.254.255.
See
> http://en.wikipedia.org/wiki/Reserved_IP_addresses.
> 
> Alternatively (and maybe better) add an isLinkLocal to the InternetAddress and
> make sure that these addresses are always last in the addresses list.

Done.

[Søren Gjesse, 2013-06-20 14:38:17]

https://codereview.chromium.org/16514010/diff/9001/runtime/bin/socket_linux.cc
File runtime/bin/socket_linux.cc (right):

https://codereview.chromium.org/16514010/diff/9001/runtime/bin/socket_linux.c...
runtime/bin/socket_linux.cc:246: (family == AF_INET || family == AF_INET6)))) {
Consider adding a static function

static bool XXX(lookup_family, family) {
return lookup_family == family ||
         (lookup_family == AF_UNSPEC &&
         (family == AF_INET || family == AF_INET6))
}

https://codereview.chromium.org/16514010/diff/9001/runtime/bin/socket_patch.dart
File runtime/bin/socket_patch.dart (right):

https://codereview.chromium.org/16514010/diff/9001/runtime/bin/socket_patch.d...
runtime/bin/socket_patch.dart:84: for (int i = 0; i < 15; i++) {
15 -> _IPV6_ADDR_LENGTH

https://codereview.chromium.org/16514010/diff/9001/runtime/bin/socket_patch.d...
runtime/bin/socket_patch.dart:87: return _sockaddr_storage[_IPV6_ADDR_OFFSET +
15] == 1;
Ditto.

https://codereview.chromium.org/16514010/diff/9001/runtime/bin/socket_patch.d...
runtime/bin/socket_patch.dart:92: switch (type) {
Please add comment here "Checking for 169.256.0.0/16"

https://codereview.chromium.org/16514010/diff/9001/runtime/bin/socket_patch.d...
runtime/bin/socket_patch.dart:97: case InternetAddressType.IP_V6:
Ditto.

[Søren Gjesse, 2013-06-20 14:38:48]

On 2013/06/20 14:38:17, Søren Gjesse wrote:
> https://codereview.chromium.org/16514010/diff/9001/runtime/bin/socket_linux.cc
> File runtime/bin/socket_linux.cc (right):
> 
>
https://codereview.chromium.org/16514010/diff/9001/runtime/bin/socket_linux.c...
> runtime/bin/socket_linux.cc:246: (family == AF_INET || family == AF_INET6))))
{
> Consider adding a static function
> 
> static bool XXX(lookup_family, family) {
> return lookup_family == family ||
>          (lookup_family == AF_UNSPEC &&
>          (family == AF_INET || family == AF_INET6))
> }
> 
>
https://codereview.chromium.org/16514010/diff/9001/runtime/bin/socket_patch.dart
> File runtime/bin/socket_patch.dart (right):
> 
>
https://codereview.chromium.org/16514010/diff/9001/runtime/bin/socket_patch.d...
> runtime/bin/socket_patch.dart:84: for (int i = 0; i < 15; i++) {
> 15 -> _IPV6_ADDR_LENGTH
> 
>
https://codereview.chromium.org/16514010/diff/9001/runtime/bin/socket_patch.d...
> runtime/bin/socket_patch.dart:87: return _sockaddr_storage[_IPV6_ADDR_OFFSET +
> 15] == 1;
> Ditto.
> 
>
https://codereview.chromium.org/16514010/diff/9001/runtime/bin/socket_patch.d...
> runtime/bin/socket_patch.dart:92: switch (type) {
> Please add comment here "Checking for 169.256.0.0/16"
> 
>
https://codereview.chromium.org/16514010/diff/9001/runtime/bin/socket_patch.d...
> runtime/bin/socket_patch.dart:97: case InternetAddressType.IP_V6:
> Ditto.

Please continue with the other platforms.

[Anders Johnsen, 2013-06-20 15:00:25]

Proceeding with other platforms.

https://codereview.chromium.org/16514010/diff/9001/runtime/bin/socket_linux.cc
File runtime/bin/socket_linux.cc (right):

https://codereview.chromium.org/16514010/diff/9001/runtime/bin/socket_linux.c...
runtime/bin/socket_linux.cc:246: (family == AF_INET || family == AF_INET6)))) {
On 2013/06/20 14:38:17, Søren Gjesse wrote:
> Consider adding a static function
> 
> static bool XXX(lookup_family, family) {
> return lookup_family == family ||
>          (lookup_family == AF_UNSPEC &&
>          (family == AF_INET || family == AF_INET6))
> }

Done.

https://codereview.chromium.org/16514010/diff/9001/runtime/bin/socket_patch.dart
File runtime/bin/socket_patch.dart (right):

https://codereview.chromium.org/16514010/diff/9001/runtime/bin/socket_patch.d...
runtime/bin/socket_patch.dart:84: for (int i = 0; i < 15; i++) {
On 2013/06/20 14:38:17, Søren Gjesse wrote:
> 15 -> _IPV6_ADDR_LENGTH

Done.

https://codereview.chromium.org/16514010/diff/9001/runtime/bin/socket_patch.d...
runtime/bin/socket_patch.dart:87: return _sockaddr_storage[_IPV6_ADDR_OFFSET +
15] == 1;
On 2013/06/20 14:38:17, Søren Gjesse wrote:
> Ditto.

Done.

https://codereview.chromium.org/16514010/diff/9001/runtime/bin/socket_patch.d...
runtime/bin/socket_patch.dart:92: switch (type) {
On 2013/06/20 14:38:17, Søren Gjesse wrote:
> Please add comment here "Checking for 169.256.0.0/16"

Done.

https://codereview.chromium.org/16514010/diff/9001/runtime/bin/socket_patch.d...
runtime/bin/socket_patch.dart:97: case InternetAddressType.IP_V6:
On 2013/06/20 14:38:17, Søren Gjesse wrote:
> Ditto.

Done.

[Anders Johnsen, 2013-06-20 17:10:37]

PTAL, added support for other platforms.

This also changes GetAddLength to take a pointer value (to avoid passing a
pointer as reference).

[Søren Gjesse, 2013-06-20 18:22:05]

LGTM!

https://codereview.chromium.org/16514010/diff/19001/runtime/bin/socket_win.cc
File runtime/bin/socket_win.cc (right):

https://codereview.chromium.org/16514010/diff/19001/runtime/bin/socket_win.cc...
runtime/bin/socket_win.cc:264:
GetAdaptersAddresses(SocketAddress::FromType(type),
You could consider adding the flags:

GAA_FLAG_SKIP_ANYCAST
GAA_FLAG_SKIP_MULTICAST
GAA_FLAG_SKIP_DNS_SERVER

https://codereview.chromium.org/16514010/diff/19001/runtime/bin/socket_win.cc...
runtime/bin/socket_win.cc:269: PIP_ADAPTER_ADDRESSES addrs =
We normally use the *. That is IP_ADAPTER_ADDRESSES* instead of
PIP_ADAPTER_ADDRESSES

https://codereview.chromium.org/16514010/diff/19001/runtime/bin/socket_win.cc...
runtime/bin/socket_win.cc:270:
reinterpret_cast<PIP_ADAPTER_ADDRESSES>(malloc(size));
Ditto and more below.

https://codereview.chromium.org/16514010/diff/19001/runtime/bin/socket_win.cc...
runtime/bin/socket_win.cc:271: // Get the addresses now we have the right
buffer.
Add an if (status == ERROR_BUFFER_OVERFLOW) around the second call to
GetAdapterAddresses.

https://codereview.chromium.org/16514010/diff/19001/runtime/bin/socket_win.h
File runtime/bin/socket_win.h (right):

https://codereview.chromium.org/16514010/diff/19001/runtime/bin/socket_win.h#...
runtime/bin/socket_win.h:13: #pragma comment(lib, "IPHLPAPI.lib")
The linking with iphlpapi.lib should be in the bin.gypi file (like ws2_32.lib).

[Anders Johnsen, 2013-06-21 07:58:35]

https://codereview.chromium.org/16514010/diff/19001/runtime/bin/socket_win.cc
File runtime/bin/socket_win.cc (right):

https://codereview.chromium.org/16514010/diff/19001/runtime/bin/socket_win.cc...
runtime/bin/socket_win.cc:264:
GetAdaptersAddresses(SocketAddress::FromType(type),
On 2013/06/20 18:22:05, Søren Gjesse wrote:
> You could consider adding the flags:
> 
> GAA_FLAG_SKIP_ANYCAST
> GAA_FLAG_SKIP_MULTICAST
> GAA_FLAG_SKIP_DNS_SERVER

Done.

https://codereview.chromium.org/16514010/diff/19001/runtime/bin/socket_win.cc...
runtime/bin/socket_win.cc:269: PIP_ADAPTER_ADDRESSES addrs =
On 2013/06/20 18:22:05, Søren Gjesse wrote:
> We normally use the *. That is IP_ADAPTER_ADDRESSES* instead of
> PIP_ADAPTER_ADDRESSES

Done.

https://codereview.chromium.org/16514010/diff/19001/runtime/bin/socket_win.cc...
runtime/bin/socket_win.cc:270:
reinterpret_cast<PIP_ADAPTER_ADDRESSES>(malloc(size));
On 2013/06/20 18:22:05, Søren Gjesse wrote:
> Ditto and more below.

Done.

https://codereview.chromium.org/16514010/diff/19001/runtime/bin/socket_win.cc...
runtime/bin/socket_win.cc:271: // Get the addresses now we have the right
buffer.
On 2013/06/20 18:22:05, Søren Gjesse wrote:
> Add an if (status == ERROR_BUFFER_OVERFLOW) around the second call to
> GetAdapterAddresses.

Done.

https://codereview.chromium.org/16514010/diff/19001/runtime/bin/socket_win.h
File runtime/bin/socket_win.h (right):

https://codereview.chromium.org/16514010/diff/19001/runtime/bin/socket_win.h#...
runtime/bin/socket_win.h:13: #pragma comment(lib, "IPHLPAPI.lib")
On 2013/06/20 18:22:05, Søren Gjesse wrote:
> The linking with iphlpapi.lib should be in the bin.gypi file (like
ws2_32.lib).

Done.

[Anders Johnsen, 2013-06-21 07:59:04]

Committed patchset #6 manually as r24267 (presubmit successful).