Fix IPAddress::ToSensitiveString() to avoid dependency on inet_ntop().

Fix IPAddress::ToSensitiveString() to avoid dependency on inet_ntop().

Previosly ToSesnsetiveString() wasn't working witn some implementations
of inet_ntop(). Rewrote it to avoid that dependency.

BUG=chromium:577344
R=pthatcher@webrtc.org, tommi@webrtc.org

Committed: https://crrev.com/beed8280d81cf1dd9a464b861f00b1ca6b70e220
Cr-Commit-Position: refs/heads/master@{#11242}

[Sergey Ulanov, 2016-01-13 22:10:49]

sergeyu@chromium.org changed reviewers:
+ henrikg@webrtc.org, tommi@webrtc.org

[Sergey Ulanov, 2016-01-13 22:15:08]

sergeyu@chromium.org changed reviewers:
+ pthatcher@webrtc.org

[Sergey Ulanov, 2016-01-13 22:15:09]

+pthatcher

[pthatcher1, 2016-01-13 22:25:23]

https://codereview.chromium.org/1584793004/diff/1/webrtc/base/ipaddress.cc
File webrtc/base/ipaddress.cc (right):

https://codereview.chromium.org/1584793004/diff/1/webrtc/base/ipaddress.cc#ne...
webrtc/base/ipaddress.cc:140: if (!rtc::inet_ntop(family_, src, buf,
sizeof(buf))) {
Would it be a good idea to remove the dependency on inet_netop altogether?

https://codereview.chromium.org/1584793004/diff/1/webrtc/base/ipaddress.cc#ne...
webrtc/base/ipaddress.cc:163: result.resize(40);
Why 40?

Truncated, it could only be 25.  Are you just playing it safe?

Also, please at least comment on it, or make a named constant for it (like
kMaxIpv6AddressStringLength).

https://codereview.chromium.org/1584793004/diff/1/webrtc/base/ipaddress.cc#ne...
webrtc/base/ipaddress.cc:169: (addr.s6_addr[4] << 8) + addr.s6_addr[5]);
Would it be worth the little extra code to make "::" instead of ":0:" in the
string?

[Sergey Ulanov, 2016-01-13 22:47:42]

https://codereview.chromium.org/1584793004/diff/1/webrtc/base/ipaddress.cc
File webrtc/base/ipaddress.cc (right):

https://codereview.chromium.org/1584793004/diff/1/webrtc/base/ipaddress.cc#ne...
webrtc/base/ipaddress.cc:140: if (!rtc::inet_ntop(family_, src, buf,
sizeof(buf))) {
On 2016/01/13 22:25:23, pthatcher1 wrote:
> Would it be a good idea to remove the dependency on inet_netop altogether?

Maybe, but I think it should be done separately from this fix.
FWIW there is an implementation of inet_ntop() in win32.cc, because inet_ntop()
didn't exist on windows before Vista.

https://codereview.chromium.org/1584793004/diff/1/webrtc/base/ipaddress.cc#ne...
webrtc/base/ipaddress.cc:163: result.resize(40);
On 2016/01/13 22:25:23, pthatcher1 wrote:
> Why 40?
> 
> Truncated, it could only be 25.  Are you just playing it safe?
> 
> Also, please at least comment on it, or make a named constant for it (like
> kMaxIpv6AddressStringLength).

Changed it to INET6_ADDRSTRLEN. (it's set to 46, for ipv4-mapped addresses)

https://codereview.chromium.org/1584793004/diff/1/webrtc/base/ipaddress.cc#ne...
webrtc/base/ipaddress.cc:169: (addr.s6_addr[4] << 8) + addr.s6_addr[5]);
On 2016/01/13 22:25:23, pthatcher1 wrote:
> Would it be worth the little extra code to make "::" instead of ":0:" in the
> string?

I don't think so. It would make this code more complicated, but it makes only
3-char difference only in some addresses and the result isn't easier to read.

[pthatcher1, 2016-01-13 22:50:44]

lgtm

[Sergey Ulanov, 2016-01-13 22:54:07]

The CQ bit was checked by sergeyu@chromium.org

[commit-bot: I haz the power, 2016-01-13 22:54:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1584793004/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1584793004/20001

[tommi, 2016-01-13 23:17:03]

lgtm

[commit-bot: I haz the power, 2016-01-14 00:54:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-01-14 00:54:32]

Try jobs failed on following builders:
  android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  linux_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  linux_compile_dbg on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  win_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[Sergey Ulanov, 2016-01-14 02:13:47]

The CQ bit was checked by sergeyu@chromium.org

[commit-bot: I haz the power, 2016-01-14 02:13:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1584793004/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1584793004/20001

[Sergey Ulanov, 2016-01-14 02:15:03]

Description was changed from

==========
Fix IPAddress::ToSensitiveString() to avoid dependency on inet_ntop().

Previosly ToSesnsetiveString() wasn't working witn some implementations
of inet_ntop(). Rewrote it to avoid that dependency.

BUG=chromium:577344
==========

to

==========
Fix IPAddress::ToSensitiveString() to avoid dependency on inet_ntop().

Previosly ToSesnsetiveString() wasn't working witn some implementations
of inet_ntop(). Rewrote it to avoid that dependency.

BUG=chromium:577344
R=pthatcher@webrtc.org, tommi@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/beed8280d81cf1dd9a464b861...
==========

[Sergey Ulanov, 2016-01-14 02:15:04]

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

[commit-bot: I haz the power, 2016-01-14 02:15:05]

Description was changed from

==========
Fix IPAddress::ToSensitiveString() to avoid dependency on inet_ntop().

Previosly ToSesnsetiveString() wasn't working witn some implementations
of inet_ntop(). Rewrote it to avoid that dependency.

BUG=chromium:577344
R=pthatcher@webrtc.org, tommi@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/beed8280d81cf1dd9a464b861...
==========

to

==========
Fix IPAddress::ToSensitiveString() to avoid dependency on inet_ntop().

Previosly ToSesnsetiveString() wasn't working witn some implementations
of inet_ntop(). Rewrote it to avoid that dependency.

BUG=chromium:577344
R=pthatcher@webrtc.org, tommi@webrtc.org

Committed: https://crrev.com/beed8280d81cf1dd9a464b861f00b1ca6b70e220
Cr-Commit-Position: refs/heads/master@{#11242}
==========

[commit-bot: I haz the power, 2016-01-14 02:15:06]

Patchset 2 (id:??) landed as
https://crrev.com/beed8280d81cf1dd9a464b861f00b1ca6b70e220
Cr-Commit-Position: refs/heads/master@{#11242}

[Henrik Grunell WebRTC, 2016-01-14 08:12:17]

lgtm