Exclude 0.0.0.0 from the list of globally routable addresses #31981
Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
| check(&[203, 2, 113, 0], false, false, false, false, true, false, false, false); | ||
| check(&[224, 0, 0, 0], false, false, false, false, true, true, false, false); | ||
| check(&[239, 255, 255, 255], false, false, false, false, true, true, false, false); | ||
| check(&[255, 255, 255, 255], false, false, false, false, false, false, true, false); |
There was a problem hiding this comment.
Why the extra space added here to all columns?
There was a problem hiding this comment.
To align with the first row, which expanded one character. If you want, I can leave this as is.
|
Could you also provide the rationale for this? When I was checking properties I unfortunately couldn't find a definition to what it meant to be a "global" IP, so if there's an RFC definition we can probably start stabilizing this as well. |
|
Global IP is taken to mean a publicly routable IP address. I did a quick search but could not find a RFC that actually defines that. My understanding is that any IP that is not reserved for a private network [1] or some special use [2] is a global IP. It might be a good idea to change the name of the function to [1] https://tools.ietf.org/html/rfc1918 |
|
The entire |
| @@ -131,7 +131,7 @@ impl Ipv4Addr { | |||
| /// - test addresses used for documentation (192.0.2.0/24, 198.51.100.0/24 and 203.0.113.0/24) | |||
|
If we're copying python in this respect then we should also at least link to the same docs |
|
@alexcrichton I added a reference to the the IANA link. Addressed all other comments. |
| /// network. This property defined by RFC 5735. | ||
| pub fn is_this_network(&self) -> bool { | ||
| self.octets()[0] == 0 | ||
| } |
There was a problem hiding this comment.
Public API addition without stability annotation?
There was a problem hiding this comment.
The whole file has a
11 #![unstable(feature = "ip", reason = "extra functionality has not been \
12 scrutinized to the level that it should \
13 be stable",
14 issue = "27709")]
Isn't that enough?
There was a problem hiding this comment.
Right, the stability part is fine then.
There was a problem hiding this comment.
Hm we're already a little unsure about including these sorts of funcitons at all, so perhaps this can be left out for now?
There was a problem hiding this comment.
@alexcrichton sorry, do you mean I should remove this function?
There was a problem hiding this comment.
Yeah let's remove it for now.
|
I might just be bikeshedding, but I'm kind of not sure about the name |
| check(&[239, 255, 255, 255], false, false, false, false, true, true, false, false); | ||
| check(&[255, 255, 255, 255], false, false, false, false, false, false, true, false); | ||
| // address unspec loopbk privt linloc global multicast brdcast doc this_net | ||
| check(&[0, 0, 0, 0], true, false, false, false, false, false, false, false, true); |
There was a problem hiding this comment.
Adding the space before the multicast column misaligns it and all of the following.
Wouldn't it be possible to remove a space before the false in the first two rows and avoid changing the whitespace in the remaining part of the table?
|
@alexcrichton please let me know if this needs more work. |
|
@alexcrichton does this look good now? |
Exclude 0.0.0.0 from the list of globally routable addresses
No description provided.