refactor!: switch to sockaddr_storage, IPv6 and resolver improvements

[Tachi107]

Using a struct sockaddr_storage allows us to get rid of the spooky union of sockaddr_in and sockaddr_in6, and to do IPv4 and IPv6 as recommended by RFC 3493.

The code is not that cleaner, but it should be more robust and less "dangerous". As discussed recently on IRC, Kip and I were quite puzzled by the various constructors of class IP, as it wasn't immediatly clear why they were memcpying INET_ADDRSTRLEN bytes into a smaller s_addr array.

Needless to say, this is a breaking change. I also removed a few now useless functions, but they could be kept if desired.

Note: this is still experimental and incomplete.

[codecov-commenter]

Codecov Report

Patch coverage: 75.80% and project coverage change: -0.39 :warning:

Comparison is base (91952a8) 78.60% compared to head (090e6a9) 78.22%.

:exclamation: Current head 090e6a9 differs from pull request most recent head 0eb0cda. Consider uploading reports for the commit 0eb0cda to get more accurate results

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #1140 +/- ## ========================================== - Coverage 78.60% 78.22% -0.39% ========================================== Files 53 53 Lines 6872 6852 -20 ========================================== - Hits 5402 5360 -42 - Misses 1470 1492 +22 ``` | [Impacted Files](https://app.codecov.io/gh/pistacheio/pistache/pull/1140?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | | |---|---|---| | [include/pistache/client.h](https://app.codecov.io/gh/pistacheio/pistache/pull/1140?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-aW5jbHVkZS9waXN0YWNoZS9jbGllbnQuaA==) | `96.77% <ø> (ø)` | | | [include/pistache/net.h](https://app.codecov.io/gh/pistacheio/pistache/pull/1140?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-aW5jbHVkZS9waXN0YWNoZS9uZXQuaA==) | `84.61% <ø> (ø)` | | | [src/client/client.cc](https://app.codecov.io/gh/pistacheio/pistache/pull/1140?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2NsaWVudC9jbGllbnQuY2M=) | `83.35% <0.00%> (-0.93%)` | :arrow_down: | | [src/server/listener.cc](https://app.codecov.io/gh/pistacheio/pistache/pull/1140?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL3NlcnZlci9saXN0ZW5lci5jYw==) | `70.21% <58.33%> (-0.74%)` | :arrow_down: | | [src/common/http\_header.cc](https://app.codecov.io/gh/pistacheio/pistache/pull/1140?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2NvbW1vbi9odHRwX2hlYWRlci5jYw==) | `92.64% <66.66%> (-0.75%)` | :arrow_down: | | [src/common/net.cc](https://app.codecov.io/gh/pistacheio/pistache/pull/1140?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2NvbW1vbi9uZXQuY2M=) | `83.05% <84.94%> (-4.59%)` | :arrow_down: | | [include/pistache/http\_header.h](https://app.codecov.io/gh/pistacheio/pistache/pull/1140?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-aW5jbHVkZS9waXN0YWNoZS9odHRwX2hlYWRlci5o) | `90.62% <100.00%> (ø)` | | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/pistacheio/pistache/pull/1140/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[Tachi107]

Yeah there are a lot of sizeofs without parentheses, I'll change them later

[Tachi107]

@kiplingw please keep this PR in draft status until I'm confident to let you merge it. I'm not done yet :)

[Tachi107]

Hey @dennisjenkins75 and @kiplingw, could you please have a look at at the changes? You can find all the relevant descriptions in the various commits.

I'm not completely done yet, I'd like to reorder the commit history, for instance.

[kiplingw]

Sure @Tachi107. Will review when back at my desk.

[dennisjenkins75]

:( I have only ever written unix sockets code the old way. I was unaware of the newer APIs. I have no problems with the code cleanup so long as it passes all unit tests.

[kiplingw]

From what I understood of it, it looks fine to me. Good work @Tachi107. Let us know when you are ready and I will re-review before merging. Main thing is, as @dennisjenkins75 said, don't break CI.

[Tachi107]

The commit history is now neatly separated in three independent commits with a somewhat exhaustive explanation about the relevant changes, and I've also added a couple more small tests. Lastly, I bumped the version to 0.2.0, since this patch introduces new features.

The commitlint and abidiff failures are expected.

@kiplingw I'll patiently wait for your re-review and merge :)

[kiplingw]

@Tachi107, BSD sockets aren't my strong point because it's been years since I had to directly use them. But from what I can understand it looks fine to me. One question I have for you though is did you try running through valgrind to see if there's any buffer overflows or other memory corruption issues before and after the changes?

[Tachi107]

Yes, I've run both under valgrind and no memory errors were raised. Note however that CI runs all tests under the memory sanitizer, which similar to valgrind in many ways.

Il 3 luglio 2023 19:46:27 CEST, Kip @.***> ha scritto:

@Tachi107, BSD sockets aren't my strong point because it's been years since I had to directly use them. But from what I can understand it looks fine to me. One question I have for you though is did you try running through valgrind to see if there's any buffer overflows or other memory corruption issues before and after the changes?

[kiplingw]

On Mon, 2023-07-03 at 13:30 -0700, Andrea Pappacoda wrote:

Yes, I've run both under valgrind and no memory errors were raised. Note however that CI runs all tests under the memory sanitizer, which similar to valgrind in many ways.

Perfect. Don't forget to bump d/changelog too.

-- Kip Warner OpenPGP signed/encrypted mail preferred https://www.thevertigo.com