net: Avoid calling getnameinfo when formatting IP addresses
ClosedPublic
Actions

Authored by PiRK on Mar 13 2023, 14:52.

Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Commits

rABC0442e823e50e: net: Avoid calling getnameinfo when formatting IP addresses

Summary

Context from the PR description and related issues:

Calling CNetAddr::ToString triggers the use of the socket syscall. The syscall is made indirectly via getnameinfo.

Not a big deal of course, but it would be nice if kept the direct and indirect use of networking related syscalls such as socket to functions where they cannot be avoided. That would make reasoning about syscall restrictions and/or application-vs-kernel boundaries somewhat easier.

https://github.com/bitcoin/bitcoin/pull/21564/commits/58580573843858068db69a378b6e9363889b1f6d

net: Add IPv4ToString (we already have IPv6ToString)

https://github.com/bitcoin/bitcoin/pull/21564/commits/58580a827d10c0675c3483e2aeca1d3ab8050ae0

net: Avoid calling getnameinfo when formatting IPv4 addresses in CNetAddr::ToStringIP

https://github.com/bitcoin/bitcoin/pull/21756/commits/c10f27fdb2d335954dd1017ce6d5800159427374

net: Make IPv6ToString do zero compression as described in RFC 5952

https://github.com/bitcoin/bitcoin/pull/21756/commits/54548bae8004a8f49d73bd29aeca8b41894214c4

net: Avoid calling getnameinfo when formatting IPv6 addresses in CNetAddr::ToStringIP

https://github.com/bitcoin/bitcoin/pull/21985/commits/6c280adcd865ae3da4df53d630c9bf737283a56f

net: Return IPv6 scope id in CNetAddr::ToStringIP()

If a scope id is provided, return it back in the string representation.
Also bring back the test.

Co-authored-by: Jon Atack <jon@atack.com>

This commit restores a feature removed in core#21756 and restores the corresponding test removed in D12175

This is a backport of core#21564, core#21756 and core#21985

Depends on D13319

Test Plan

ninja all check-all

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

PiRK created this revision.Mar 13 2023, 14:52

Herald added a reviewer: Restricted Project. · View Herald TranscriptMar 13 2023, 14:52

PiRK requested review of this revision.Mar 13 2023, 14:52

teamcity edited the summary of this revision. (Show Details)Mar 13 2023, 14:52

PiRK added inline comments.Mar 13 2023, 14:56

.arclint
49 ↗	(On Diff #38504)	The false positive is on line 622 in netaddress.cpp.
src/netaddress.cpp
572 ↗	(On Diff #38504)	The deviation from source material here is to avoid a `-Wmissing-braces` warning (single pair of braces) or an error `error: no matching function for call to ‘array(<brace-enclosed initializer list>)’` (double pair of braces without specifying the data type and array length) Also there is no other place in the codebase where we declare a std::array without specifying the type of data and array length.

Harbormaster completed remote builds in B22408: Diff 38504.Mar 13 2023, 15:18

Fabien requested changes to this revision.Mar 13 2023, 15:43

Fabien added a subscriber: Fabien.

Fabien added inline comments.

.arclint
49 ↗	(On Diff #38504)	This should not be in the .arclint, as this is a single exception. The best course of action is to fix the case, as it exhibits a limitation of the linter (not managing %%). The worst is to add an exception in the linter itself, but not exclude the whole file.