Add a mDNS package

Add a mDNS package

Implementation of mDNS one-shot query.

This package uses dart:io and runs only on the Dart VM.

R=karlklose@google.com
BUG=

Committed: https://github.com/dart-lang/fletch/commit/f0d30ec90e9c6c9c157fa583c74a5f8d5321891d

[karlklose, 2015-10-28 10:14:31]

LGTM.

https://codereview.chromium.org/1426863003/diff/1/pkg/mdns/lib/mdns.dart
File pkg/mdns/lib/mdns.dart (right):

https://codereview.chromium.org/1426863003/diff/1/pkg/mdns/lib/mdns.dart#newc...
pkg/mdns/lib/mdns.dart:18: class MDNSClient {
I think this should be 'MDnsClient'.

https://codereview.chromium.org/1426863003/diff/1/pkg/mdns/lib/mdns.dart#newc...
pkg/mdns/lib/mdns.dart:22: final List<RawDatagramSocket> _sockets = [];
Consider adding a type to the list literal to catch type errors at runtime.

https://codereview.chromium.org/1426863003/diff/1/pkg/mdns/lib/mdns.dart#newc...
pkg/mdns/lib/mdns.dart:24: final Duration _defaultTimeout = new
Duration(seconds: 5);
Does it make sense to remove this field (it cannot be altered anyway) and make
'const Duration(seconds: 5)' the default value for timeout in  lookup?

https://codereview.chromium.org/1426863003/diff/1/pkg/mdns/lib/mdns.dart#newc...
pkg/mdns/lib/mdns.dart:26: MDNSClient();
Not needed.

https://codereview.chromium.org/1426863003/diff/1/pkg/mdns/lib/mdns.dart#newc...
pkg/mdns/lib/mdns.dart:42: var socket;
Move declaration to line 45?

https://codereview.chromium.org/1426863003/diff/1/pkg/mdns/lib/mdns.dart#newc...
pkg/mdns/lib/mdns.dart:43: for (int i = 0; i < interfaces.length; i++) {
How about:

  for (NetworkInterface interface in interfaces) {
    ...
  }
?

https://codereview.chromium.org/1426863003/diff/1/pkg/mdns/lib/mdns.dart#newc...
pkg/mdns/lib/mdns.dart:60: if (!_started) return;
Should this method throw if '_starting == true'?

https://codereview.chromium.org/1426863003/diff/1/pkg/mdns/lib/src/constants....
File pkg/mdns/lib/src/constants.dart (right):

https://codereview.chromium.org/1426863003/diff/1/pkg/mdns/lib/src/constants....
pkg/mdns/lib/src/constants.dart:9: InternetAddress mDNSAddress = new
InternetAddress('224.0.0.251');
'mDnsAddress'

https://codereview.chromium.org/1426863003/diff/1/pkg/mdns/lib/src/constants....
pkg/mdns/lib/src/constants.dart:10: const int mDNSPort = 5353;
'mDnsPort'

https://codereview.chromium.org/1426863003/diff/1/pkg/mdns/lib/src/packet.dart
File pkg/mdns/lib/src/packet.dart (right):

https://codereview.chromium.org/1426863003/diff/1/pkg/mdns/lib/src/packet.dar...
pkg/mdns/lib/src/packet.dart:29: bd.setUint16(qdcountOffset, 1);
Maybe add comments to describe the meaning of the field (here or at the constant
definitions).

https://codereview.chromium.org/1426863003/diff/1/pkg/mdns/lib/src/packet.dar...
pkg/mdns/lib/src/packet.dart:57: /// If decoding fails (e.g. die to an invalid
packet) `null` is returned.
'die' -> 'due'.

https://codereview.chromium.org/1426863003/diff/1/pkg/mdns/lib/src/packet.dar...
pkg/mdns/lib/src/packet.dart:83: List readFQDN(int offset) {
I would prefer a few more type annotations on/in this method.

[Søren Gjesse, 2015-10-28 11:36:57]

https://codereview.chromium.org/1426863003/diff/1/pkg/mdns/lib/mdns.dart
File pkg/mdns/lib/mdns.dart (right):

https://codereview.chromium.org/1426863003/diff/1/pkg/mdns/lib/mdns.dart#newc...
pkg/mdns/lib/mdns.dart:18: class MDNSClient {
On 2015/10/28 10:14:30, karlklose wrote:
> I think this should be 'MDnsClient'.

Yes, you are probably right.

https://codereview.chromium.org/1426863003/diff/1/pkg/mdns/lib/mdns.dart#newc...
pkg/mdns/lib/mdns.dart:22: final List<RawDatagramSocket> _sockets = [];
On 2015/10/28 10:14:30, karlklose wrote:
> Consider adding a type to the list literal to catch type errors at runtime.

Done.

https://codereview.chromium.org/1426863003/diff/1/pkg/mdns/lib/mdns.dart#newc...
pkg/mdns/lib/mdns.dart:24: final Duration _defaultTimeout = new
Duration(seconds: 5);
On 2015/10/28 10:14:30, karlklose wrote:
> Does it make sense to remove this field (it cannot be altered anyway) and make
> 'const Duration(seconds: 5)' the default value for timeout in  lookup?

Absolutely. Done.

https://codereview.chromium.org/1426863003/diff/1/pkg/mdns/lib/mdns.dart#newc...
pkg/mdns/lib/mdns.dart:26: MDNSClient();
On 2015/10/28 10:14:30, karlklose wrote:
> Not needed.

Removed.

https://codereview.chromium.org/1426863003/diff/1/pkg/mdns/lib/mdns.dart#newc...
pkg/mdns/lib/mdns.dart:42: var socket;
On 2015/10/28 10:14:30, karlklose wrote:
> Move declaration to line 45?

Done.

https://codereview.chromium.org/1426863003/diff/1/pkg/mdns/lib/mdns.dart#newc...
pkg/mdns/lib/mdns.dart:43: for (int i = 0; i < interfaces.length; i++) {
On 2015/10/28 10:14:30, karlklose wrote:
> How about:
> 
>   for (NetworkInterface interface in interfaces) {
>     ...
>   }
> ?

Sure, done.

https://codereview.chromium.org/1426863003/diff/1/pkg/mdns/lib/mdns.dart#newc...
pkg/mdns/lib/mdns.dart:60: if (!_started) return;
On 2015/10/28 10:14:30, karlklose wrote:
> Should this method throw if '_starting == true'?

Done.

https://codereview.chromium.org/1426863003/diff/1/pkg/mdns/lib/src/constants....
File pkg/mdns/lib/src/constants.dart (right):

https://codereview.chromium.org/1426863003/diff/1/pkg/mdns/lib/src/constants....
pkg/mdns/lib/src/constants.dart:9: InternetAddress mDNSAddress = new
InternetAddress('224.0.0.251');
On 2015/10/28 10:14:30, karlklose wrote:
> 'mDnsAddress'

Done.

https://codereview.chromium.org/1426863003/diff/1/pkg/mdns/lib/src/constants....
pkg/mdns/lib/src/constants.dart:10: const int mDNSPort = 5353;
On 2015/10/28 10:14:30, karlklose wrote:
> 'mDnsPort'

Done.

https://codereview.chromium.org/1426863003/diff/1/pkg/mdns/lib/src/packet.dart
File pkg/mdns/lib/src/packet.dart (right):

https://codereview.chromium.org/1426863003/diff/1/pkg/mdns/lib/src/packet.dar...
pkg/mdns/lib/src/packet.dart:29: bd.setUint16(qdcountOffset, 1);
On 2015/10/28 10:14:30, karlklose wrote:
> Maybe add comments to describe the meaning of the field (here or at the
constant
> definitions).

Done.

https://codereview.chromium.org/1426863003/diff/1/pkg/mdns/lib/src/packet.dar...
pkg/mdns/lib/src/packet.dart:57: /// If decoding fails (e.g. die to an invalid
packet) `null` is returned.
On 2015/10/28 10:14:31, karlklose wrote:
> 'die' -> 'due'.

Done.

https://codereview.chromium.org/1426863003/diff/1/pkg/mdns/lib/src/packet.dar...
pkg/mdns/lib/src/packet.dart:83: List readFQDN(int offset) {
On 2015/10/28 10:14:30, karlklose wrote:
> I would prefer a few more type annotations on/in this method.

Added a type FQDNReadResult instead of using a two element list.

Added type to parts.

[Søren Gjesse, 2015-10-28 11:42:18]

Committed patchset #2 (id:20001) manually as
f0d30ec90e9c6c9c157fa583c74a5f8d5321891d (presubmit successful).

[karlklose, 2015-10-29 10:29:44]

https://codereview.chromium.org/1426863003/diff/20001/pkg/mdns/lib/src/packet...
File pkg/mdns/lib/src/packet.dart (right):

https://codereview.chromium.org/1426863003/diff/20001/pkg/mdns/lib/src/packet...
pkg/mdns/lib/src/packet.dart:72: /// See See https://tools.ietf.org/html/rfc1035
for the format.
'See See' -> 'See'.