Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1408803010:
Add IPAddress class as a replacement for the IPAddressNumber typedef. (Closed)
DescriptionThis CL adds an IPAddress class intended to replace the current IPAddressNumber typedef. The class wraps a std::vector<uint8_t> containing the actual IP address and exposes a couple of helper methods. This CL only introduces the new class and related tests.
BUG=496258
Committed: https://crrev.com/04e01ee14b56c54d2fa59ee0b5c35e727669461b
Cr-Commit-Position: refs/heads/master@{#363046}
Patch Set 1 : Rename to IPAddress #Patch Set 2 : #
Total comments: 18
Patch Set 3 : Comments eroman #
Total comments: 23
Patch Set 4 : Comments eroman #
Total comments: 12
Patch Set 5 : Remove duplicated implementations #
Total comments: 1
Messages
Total messages: 29 (15 generated)
Patchset #2 (id:20001) has been deleted
Patchset #2 (id:40001) has been deleted
Patchset #1 (id:1) has been deleted
Description was changed from ========== Add IPAddressNumberNew as a replacement for IPAdressNumber. BUG=496258 ========== to ========== This adds an IPAddress class intented to replace the current IPAddressNumber typedef. The class wraps a std::vector<uint8_t> containing the actual IP address and exposes methods to work with the encapsulated address (like the functions currently in ip_address_number.h). This CL only introduces the new class and related tests. As a starting point for the migration, it also updates IPEndPoint to make it accept both IPAddress and IPAddressNumber instances. After looking up IPAddressNumber usage with codesearch it seems a lot of instance and up being used by IPEndPoint, so I thought that would be a good place to start. (Would it be better to migrate IPAddressNumber usage to the new class in one CL?) BUG=496258 ==========
martijn@martijnc.be changed reviewers: + eroman@chromium.org, mmenke@chromium.org
Patchset #2 (id:80001) has been deleted
Description was changed from ========== This adds an IPAddress class intented to replace the current IPAddressNumber typedef. The class wraps a std::vector<uint8_t> containing the actual IP address and exposes methods to work with the encapsulated address (like the functions currently in ip_address_number.h). This CL only introduces the new class and related tests. As a starting point for the migration, it also updates IPEndPoint to make it accept both IPAddress and IPAddressNumber instances. After looking up IPAddressNumber usage with codesearch it seems a lot of instance and up being used by IPEndPoint, so I thought that would be a good place to start. (Would it be better to migrate IPAddressNumber usage to the new class in one CL?) BUG=496258 ========== to ========== This CL adds an IPAddress class intented to replace the current IPAddressNumber typedef. The class wraps a std::vector<unsigned char> containing the actual IP address and exposes methods to work with the encapsulated address (like the functions currently in ip_address_number.h). This CL only introduces the new class and related tests. As a starting point for the migration, it also updates IPEndPoint to make it accept both IPAddress and IPAddressNumber instances. After looking up IPAddressNumber usage with codesearch it seems a lot of instance and up being used by IPEndPoint, so I thought that would be a good place to start. BUG=496258 ==========
martijn@martijnc.be changed reviewers: - mmenke@chromium.org
Hi, I created an IPAddress class as a possible replacement for IPAddressNumber. Can you take a look? Thanks! This CL doesn't migrate any consumers yet but I was thinking to start with IPEndPoint consumers (in separate CL's). Or would it be better to migrate IPAddressNumber consumers to the new class in one CL?
Thanks for working on this!
Some general design comments before going into too much detail (@mmenke feel
free to sanity check my design suggestions):
This change is moving all of the utility functions from ip_address_number.h to
being member functions of IPAddress. I don't think that is a good thing to do
here for two reasons:
(1) It makes it hard to review the actual code changes (a lot of moves are
showing up as additions). Also while transitioning code this is resulting in
duplication.
(2) Not all of those methods seem general enough to be part of IPAddress class
(at least in my opinion, but that is a debatable stylistic thing). From a future
extensibility perspective I don't think we want to necessarily dump all
functions that operate on IPAddresses into the class, just the most essential
ones.
The way I suggest organizing this change is to instead start off with a minimal
IPAddress implementation. It would have these methods:
* Default constructor that creates null/empty IPAddress.
* A constructor that takes a pointer to bytes and a size (and copies into its
internal vector)
* The various Is*() methods for testing ipv4/ipv6/reserved/mapped.
* The ToString() method
* A FromIPLiteral() factory method
* operator==, operator<
* A function to get at the underlying byte vector, say:
const std::vector<uint8_t>& bytes() const;
The rest of the utility methods in ip_address_number stay in
ip_address_number.cc/h without any changes.
As we update consumers to store an IPAddress in place of an IPAddressNumber, we
may add overloads to the utility methods that take an IPAddress rather than
IPAddressNumber, and then just grab .bytes().
The final phase, once everything has been transitioned over to IPAddress would
be to remove the unnecessary utility method overloads, so that most things take
const IPAddress&.
If needed, we can expose the ip address inputs to the utility methods via a byte
slice (i.e. StringPiece or pointer+size) to make it possible to call them on
general data, without having to fist copy into an IPAddress. In practice I don't
think we will have much demand for that if IPAddress is being used throughout,
but we can evaluate later.
https://codereview.chromium.org/1408803010/diff/100001/net/base/ip_address.h
File net/base/ip_address.h (right):
https://codereview.chromium.org/1408803010/diff/100001/net/base/ip_address.h#...
net/base/ip_address.h:12: #include "base/gtest_prod_util.h"
Why is this included? (I see only "friend" in the implementation, and no
FRIEND_TEST).
https://codereview.chromium.org/1408803010/diff/100001/net/base/ip_address.h#...
net/base/ip_address.h:23: IPAddress();
Please document what default construction does (will be a zero-sized address,
that is invalid).
https://codereview.chromium.org/1408803010/diff/100001/net/base/ip_address.h#...
net/base/ip_address.h:26: static const size_t kIPv4AddressSize = 4;
I believe the conclusion in
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/hDqJ4KBVqog
was that we shouldn't provide the definition inline.
Instead move the definition to the .cc file.
https://codereview.chromium.org/1408803010/diff/100001/net/base/ip_address.h#...
net/base/ip_address.h:43: // Returns the string representation of an IP address.
nit: Instead of "the string representation" can you say "the canonical string
representation" or "a string representation". There are many different ways to
represent both IPv4 and especially IPv6 literals, the one we use is what GURL
thinks is canonical.
https://codereview.chromium.org/1408803010/diff/100001/net/base/ip_address.h#...
net/base/ip_address.h:49: std::string ToStringWithPort(uint16_t port) const;
I don't think this function belongs here, as this is more so functionality for
IPEndPoint.
https://codereview.chromium.org/1408803010/diff/100001/net/base/ip_address.h#...
net/base/ip_address.h:52: std::string ToPackedString() const;
Rather than copy into a string, why not something like:
const std::vector<uinit8_t> bytes() const { return ip_address_; }
(Also removes the need for friending IPEndPoint
https://codereview.chromium.org/1408803010/diff/100001/net/base/ip_address.h#...
net/base/ip_address.h:129: bool operator>(const IPAddress& that) const;
Why provide != and > ?
https://codereview.chromium.org/1408803010/diff/100001/net/base/ip_address.h#...
net/base/ip_address.h:148: std::vector<unsigned char> ip_address_;
uint8_t
https://codereview.chromium.org/1408803010/diff/100001/net/base/ip_address.h#...
net/base/ip_address.h:150: static const unsigned char kIPv4MappedPrefix[];
This can go into the .cc file as either a static variable, or in an anonymous
namespace instead.
Patchset #3 (id:120001) has been deleted
Patchset #3 (id:140001) has been deleted
Description was changed from ========== This CL adds an IPAddress class intented to replace the current IPAddressNumber typedef. The class wraps a std::vector<unsigned char> containing the actual IP address and exposes methods to work with the encapsulated address (like the functions currently in ip_address_number.h). This CL only introduces the new class and related tests. As a starting point for the migration, it also updates IPEndPoint to make it accept both IPAddress and IPAddressNumber instances. After looking up IPAddressNumber usage with codesearch it seems a lot of instance and up being used by IPEndPoint, so I thought that would be a good place to start. BUG=496258 ========== to ========== This CL adds an IPAddress class intended to replace the current IPAddressNumber typedef. The class wraps a std::vector<uint8_t> containing the actual IP address and exposes a couple of helper methods. This CL only introduces the new class and related tests. BUG=496258 ==========
Thank you for the review and design comments! I've uploaded a new patch that addresses them. https://codereview.chromium.org/1408803010/diff/100001/net/base/ip_address.h File net/base/ip_address.h (right): https://codereview.chromium.org/1408803010/diff/100001/net/base/ip_address.h#... net/base/ip_address.h:12: #include "base/gtest_prod_util.h" On 2015/11/24 at 00:21:49, eroman wrote: > Why is this included? (I see only "friend" in the implementation, and no FRIEND_TEST). Removed. (Was leftover from some earlier testing) https://codereview.chromium.org/1408803010/diff/100001/net/base/ip_address.h#... net/base/ip_address.h:23: IPAddress(); On 2015/11/24 at 00:21:49, eroman wrote: > Please document what default construction does (will be a zero-sized address, that is invalid). Done. https://codereview.chromium.org/1408803010/diff/100001/net/base/ip_address.h#... net/base/ip_address.h:26: static const size_t kIPv4AddressSize = 4; On 2015/11/24 at 00:21:49, eroman wrote: > I believe the conclusion in https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/hDqJ4KBVqog was that we shouldn't provide the definition inline. > > Instead move the definition to the .cc file. Done. https://codereview.chromium.org/1408803010/diff/100001/net/base/ip_address.h#... net/base/ip_address.h:43: // Returns the string representation of an IP address. On 2015/11/24 at 00:21:49, eroman wrote: > nit: Instead of "the string representation" can you say "the canonical string representation" or "a string representation". There are many different ways to represent both IPv4 and especially IPv6 literals, the one we use is what GURL thinks is canonical. Done. https://codereview.chromium.org/1408803010/diff/100001/net/base/ip_address.h#... net/base/ip_address.h:49: std::string ToStringWithPort(uint16_t port) const; On 2015/11/24 at 00:21:49, eroman wrote: > I don't think this function belongs here, as this is more so functionality for IPEndPoint. Removed. https://codereview.chromium.org/1408803010/diff/100001/net/base/ip_address.h#... net/base/ip_address.h:52: std::string ToPackedString() const; On 2015/11/24 at 00:21:49, eroman wrote: > Rather than copy into a string, why not something like: > > const std::vector<uinit8_t> bytes() const { return ip_address_; } > > (Also removes the need for friending IPEndPoint Done (removed). https://codereview.chromium.org/1408803010/diff/100001/net/base/ip_address.h#... net/base/ip_address.h:129: bool operator>(const IPAddress& that) const; On 2015/11/24 at 00:21:49, eroman wrote: > Why provide != and > ? Removed. (added them initially for completeness) https://codereview.chromium.org/1408803010/diff/100001/net/base/ip_address.h#... net/base/ip_address.h:148: std::vector<unsigned char> ip_address_; On 2015/11/24 at 00:21:49, eroman wrote: > uint8_t Done. https://codereview.chromium.org/1408803010/diff/100001/net/base/ip_address.h#... net/base/ip_address.h:150: static const unsigned char kIPv4MappedPrefix[]; On 2015/11/24 at 00:21:49, eroman wrote: > This can go into the .cc file as either a static variable, or in an anonymous namespace instead. Done.
The design looks good! Please also add a TODO comment in ip_address_number.h that new code should use IPAddress, and link to the bug number. https://codereview.chromium.org/1408803010/diff/160001/net/base/ip_address.cc File net/base/ip_address.cc (right): https://codereview.chromium.org/1408803010/diff/160001/net/base/ip_address.cc... net/base/ip_address.cc:66: size_t address_len = ip_address_.size(); I suggest removing this temporary variable, and just calling ip_address_.size() below. https://codereview.chromium.org/1408803010/diff/160001/net/base/ip_address.cc... net/base/ip_address.cc:92: ip_address->ip_address_ = ip; (1) only replace ip_address_ on success (2) use std::swap() to make it more efficient. if (!url::IPV6AddressToNumber....) return false; std::swap(ip, ip_address->ip_address_); https://codereview.chromium.org/1408803010/diff/160001/net/base/ip_address.cc... net/base/ip_address.cc:103: ip_address->ip_address_ = ip; same thing here. https://codereview.chromium.org/1408803010/diff/160001/net/base/ip_address.h File net/base/ip_address.h (right): https://codereview.chromium.org/1408803010/diff/160001/net/base/ip_address.h#... net/base/ip_address.h:24: IPAddress(const uint8_t* address, size_t address_len); Please add a comment explaining this creates a copy, and the input address is expected to be in byte order. https://codereview.chromium.org/1408803010/diff/160001/net/base/ip_address.h#... net/base/ip_address.h:45: // For example: "192.168.0.1" or "::1". Please also add a mention that it is incorrect to call this with an invalid address (it will trigger a crash). https://codereview.chromium.org/1408803010/diff/160001/net/base/ip_address.h#... net/base/ip_address.h:60: friend class IPAddressTest; Delete, I don't believe this is needed anymore. https://codereview.chromium.org/1408803010/diff/160001/net/base/ip_address_un... File net/base/ip_address_unittest.cc (right): https://codereview.chromium.org/1408803010/diff/160001/net/base/ip_address_un... net/base/ip_address_unittest.cc:14: class IPAddressTest : public testing::Test { Note that this class is a test fixture, however it is unused (none of the tests below are TEST_F). There doesn't seem to be any need for this class, instead make all its static members into regular functions. https://codereview.chromium.org/1408803010/diff/160001/net/base/ip_address_un... net/base/ip_address_unittest.cc:16: // Helper to strignize an IP number (used to define expectations). typo: stringize https://codereview.chromium.org/1408803010/diff/160001/net/base/ip_address_un... net/base/ip_address_unittest.cc:22: out.append(base::IntToString(static_cast<int>(v.ip_address_[i]))); I realize this is moving existing code, however can you change this to remove the cast: base::UintToString(v.ip_address_[i]); https://codereview.chromium.org/1408803010/diff/160001/net/base/ip_address_un... net/base/ip_address_unittest.cc:27: static IPAddress ArrayToIPAdress(uint8_t address[], size_t length) { How about instead using some template magic: template <size_t N> static IPAddress ArrayToIPAddress(const uint8_t(&address)[N]) { return IPAddress(address, N); } Then callers can call it without needing the sizeof(): const addr1[] = {192, 168, 0, 1}; IPAddress ip_address1 = ArrayToIPAddress(addr1); https://codereview.chromium.org/1408803010/diff/160001/net/base/ip_address_un... net/base/ip_address_unittest.cc:33: namespace { Move this up to contain the helper functions above.
https://codereview.chromium.org/1408803010/diff/160001/net/base/ip_address.h File net/base/ip_address.h (right): https://codereview.chromium.org/1408803010/diff/160001/net/base/ip_address.h#... net/base/ip_address.h:24: IPAddress(const uint8_t* address, size_t address_len); On 2015/11/30 22:12:33, eroman wrote: > Please add a comment explaining this creates a copy, and the input address is > expected to be in byte order. By "byte order" I really meant to say "network byte order" :)
Patchset #4 (id:180001) has been deleted
Patchset #4 (id:200001) has been deleted
Thank you for the review! I've uploaded a new patch. > Please also add a TODO comment in ip_address_number.h that new code should use IPAddress, and link to the bug number. Done.
https://codereview.chromium.org/1408803010/diff/160001/net/base/ip_address.cc File net/base/ip_address.cc (right): https://codereview.chromium.org/1408803010/diff/160001/net/base/ip_address.cc... net/base/ip_address.cc:66: size_t address_len = ip_address_.size(); On 2015/11/30 at 22:12:33, eroman wrote: > I suggest removing this temporary variable, and just calling ip_address_.size() below. Done. https://codereview.chromium.org/1408803010/diff/160001/net/base/ip_address.cc... net/base/ip_address.cc:92: ip_address->ip_address_ = ip; On 2015/11/30 at 22:12:33, eroman wrote: > (1) only replace ip_address_ on success > (2) use std::swap() to make it more efficient. > > if (!url::IPV6AddressToNumber....) > return false; > > std::swap(ip, ip_address->ip_address_); Done. https://codereview.chromium.org/1408803010/diff/160001/net/base/ip_address.cc... net/base/ip_address.cc:103: ip_address->ip_address_ = ip; On 2015/11/30 at 22:12:33, eroman wrote: > same thing here. Done. https://codereview.chromium.org/1408803010/diff/160001/net/base/ip_address.h File net/base/ip_address.h (right): https://codereview.chromium.org/1408803010/diff/160001/net/base/ip_address.h#... net/base/ip_address.h:24: IPAddress(const uint8_t* address, size_t address_len); On 2015/11/30 at 22:12:33, eroman wrote: > Please add a comment explaining this creates a copy, and the input address is expected to be in byte order. Done. https://codereview.chromium.org/1408803010/diff/160001/net/base/ip_address.h#... net/base/ip_address.h:45: // For example: "192.168.0.1" or "::1". On 2015/11/30 at 22:12:33, eroman wrote: > Please also add a mention that it is incorrect to call this with an invalid address (it will trigger a crash). Done. https://codereview.chromium.org/1408803010/diff/160001/net/base/ip_address.h#... net/base/ip_address.h:60: friend class IPAddressTest; On 2015/11/30 at 22:12:33, eroman wrote: > Delete, I don't believe this is needed anymore. Done. https://codereview.chromium.org/1408803010/diff/160001/net/base/ip_address_un... File net/base/ip_address_unittest.cc (right): https://codereview.chromium.org/1408803010/diff/160001/net/base/ip_address_un... net/base/ip_address_unittest.cc:14: class IPAddressTest : public testing::Test { On 2015/11/30 at 22:12:33, eroman wrote: > Note that this class is a test fixture, however it is unused (none of the tests below are TEST_F). > > There doesn't seem to be any need for this class, instead make all its static members into regular functions. Done. https://codereview.chromium.org/1408803010/diff/160001/net/base/ip_address_un... net/base/ip_address_unittest.cc:16: // Helper to strignize an IP number (used to define expectations). On 2015/11/30 at 22:12:33, eroman wrote: > typo: stringize Done. https://codereview.chromium.org/1408803010/diff/160001/net/base/ip_address_un... net/base/ip_address_unittest.cc:22: out.append(base::IntToString(static_cast<int>(v.ip_address_[i]))); On 2015/11/30 at 22:12:33, eroman wrote: > I realize this is moving existing code, however can you change this to remove the cast: > > base::UintToString(v.ip_address_[i]); Done. https://codereview.chromium.org/1408803010/diff/160001/net/base/ip_address_un... net/base/ip_address_unittest.cc:27: static IPAddress ArrayToIPAdress(uint8_t address[], size_t length) { On 2015/11/30 at 22:12:33, eroman wrote: > How about instead using some template magic: > > template <size_t N> > static IPAddress ArrayToIPAddress(const uint8_t(&address)[N]) { > return IPAddress(address, N); > } > > Then callers can call it without needing the sizeof(): > > const addr1[] = {192, 168, 0, 1}; > IPAddress ip_address1 = ArrayToIPAddress(addr1); Done. https://codereview.chromium.org/1408803010/diff/160001/net/base/ip_address_un... net/base/ip_address_unittest.cc:33: namespace { On 2015/11/30 at 22:12:33, eroman wrote: > Move this up to contain the helper functions above. Done.
https://codereview.chromium.org/1408803010/diff/220001/net/base/ip_address.cc File net/base/ip_address.cc (right): https://codereview.chromium.org/1408803010/diff/220001/net/base/ip_address.cc... net/base/ip_address.cc:14: const unsigned char kIPv4MappedPrefix[] = {0, 0, 0, 0, 0, 0, Can remove this per the later comment. https://codereview.chromium.org/1408803010/diff/220001/net/base/ip_address.cc... net/base/ip_address.cc:26: : ip_address_(std::vector<uint8_t>(address, address + address_len)) {} std::vector<> is not needed here. How about just: : ip_address_(address, address + address_len) https://codereview.chromium.org/1408803010/diff/220001/net/base/ip_address.cc... net/base/ip_address.cc:36: // Don't compare IPv4 and IPv6 addresses (they have different range Remove this comment (it is duplicated from ip_address_number.cc). Since we are just calling that other implementation no need for it. https://codereview.chromium.org/1408803010/diff/220001/net/base/ip_address.cc... net/base/ip_address.cc:49: bool IPAddress::IsIPv4Mapped() const { Sorry I should have been more explicit about this in the earlier patchset, but I don't want this change to duplicate existing code in ip_address_number.cc There will be a transition period where functionality will have entry points in both places (Method in IPAddress, and free-floating function in ip_address_number.h). During this time we don't want to have duplicated implementations, but instead just "forward" the implementation. In the future when the callers have been converted to using IPAddress and the free-floating function is no longer being used, we can move it to this .cc file. For this CL we can simply do: return IsIpv4Mapped(ip_address_); https://codereview.chromium.org/1408803010/diff/220001/net/base/ip_address.cc... net/base/ip_address.cc:57: std::string IPAddress::ToString() const { Same thing here: return IPAddressToString(ip_address_); https://codereview.chromium.org/1408803010/diff/220001/net/base/ip_address.cc... net/base/ip_address.cc:74: bool IPAddress::FromIPLiteral(const base::StringPiece& ip_literal, Same here: std::vector<uint8_t> number; if (!ParseIPLiteralToNumber(ip_literal, &number)) return false; std::swap(number, ip_address->ip_address_); return true;
Thank you for the review. I've removed the duplicated code (left the tests in, is that OK?). https://codereview.chromium.org/1408803010/diff/220001/net/base/ip_address.cc File net/base/ip_address.cc (right): https://codereview.chromium.org/1408803010/diff/220001/net/base/ip_address.cc... net/base/ip_address.cc:14: const unsigned char kIPv4MappedPrefix[] = {0, 0, 0, 0, 0, 0, On 2015/12/01 at 23:00:39, eroman wrote: > Can remove this per the later comment. Done. https://codereview.chromium.org/1408803010/diff/220001/net/base/ip_address.cc... net/base/ip_address.cc:26: : ip_address_(std::vector<uint8_t>(address, address + address_len)) {} On 2015/12/01 at 23:00:40, eroman wrote: > std::vector<> is not needed here. How about just: > > : ip_address_(address, address + address_len) Done. https://codereview.chromium.org/1408803010/diff/220001/net/base/ip_address.cc... net/base/ip_address.cc:36: // Don't compare IPv4 and IPv6 addresses (they have different range On 2015/12/01 at 23:00:39, eroman wrote: > Remove this comment (it is duplicated from ip_address_number.cc). > > Since we are just calling that other implementation no need for it. Done. https://codereview.chromium.org/1408803010/diff/220001/net/base/ip_address.cc... net/base/ip_address.cc:49: bool IPAddress::IsIPv4Mapped() const { On 2015/12/01 at 23:00:40, eroman wrote: > Sorry I should have been more explicit about this in the earlier patchset, but I don't want this change to duplicate existing code in ip_address_number.cc > > There will be a transition period where functionality will have entry points in both places (Method in IPAddress, and free-floating function in ip_address_number.h). During this time we don't want to have duplicated implementations, but instead just "forward" the implementation. > > In the future when the callers have been converted to using IPAddress and the free-floating function is no longer being used, we can move it to this .cc file. > > For this CL we can simply do: > > return IsIpv4Mapped(ip_address_); Done. https://codereview.chromium.org/1408803010/diff/220001/net/base/ip_address.cc... net/base/ip_address.cc:57: std::string IPAddress::ToString() const { On 2015/12/01 at 23:00:40, eroman wrote: > Same thing here: > > return IPAddressToString(ip_address_); Done. https://codereview.chromium.org/1408803010/diff/220001/net/base/ip_address.cc... net/base/ip_address.cc:74: bool IPAddress::FromIPLiteral(const base::StringPiece& ip_literal, On 2015/12/01 at 23:00:40, eroman wrote: > Same here: > > std::vector<uint8_t> number; > if (!ParseIPLiteralToNumber(ip_literal, &number)) > return false; > > std::swap(number, ip_address->ip_address_); > return true; Done.
lgtm
The CQ bit was checked by martijn@martijnc.be
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1408803010/240001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1408803010/240001
Message was sent while issue was closed.
Committed patchset #5 (id:240001)
Message was sent while issue was closed.
Description was changed from ========== This CL adds an IPAddress class intended to replace the current IPAddressNumber typedef. The class wraps a std::vector<uint8_t> containing the actual IP address and exposes a couple of helper methods. This CL only introduces the new class and related tests. BUG=496258 ========== to ========== This CL adds an IPAddress class intended to replace the current IPAddressNumber typedef. The class wraps a std::vector<uint8_t> containing the actual IP address and exposes a couple of helper methods. This CL only introduces the new class and related tests. BUG=496258 Committed: https://crrev.com/04e01ee14b56c54d2fa59ee0b5c35e727669461b Cr-Commit-Position: refs/heads/master@{#363046} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/04e01ee14b56c54d2fa59ee0b5c35e727669461b Cr-Commit-Position: refs/heads/master@{#363046}
Message was sent while issue was closed.
https://codereview.chromium.org/1408803010/diff/240001/net/base/ip_address.h File net/base/ip_address.h (right): https://codereview.chromium.org/1408803010/diff/240001/net/base/ip_address.h#... net/base/ip_address.h:10: #include "base/basictypes.h" hum, we should include <stdint.h> instead now. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
