tezc / sc

Common libraries and data structures for C.
BSD 3-Clause "New" or "Revised" License
2.23k stars 239 forks source link

Function sc_sock_connect() picks wrong address faimily #87

Closed svladykin closed 2 years ago

svladykin commented 2 years ago

Connecting in non-blocking mode AF_INET6 socket with sc_sock_connect("localhost") where both IPv4 and IPv6 addresses are assigned to "localhost" but the server listens on IPv6 only.

It fails with Connection refused because getaddrinfo looks for any available address with AF_UNSPEC and wrongly picks IPv4. And since it is non-blocking mode it can not retry after the failure.

This change makes sc_sock_connect() give more respect to the configured socket family: the first iteration we try to connect to addresses with the matching family, the second iteration we try to connect to the rest.

svladykin commented 2 years ago

The diff looks horrible, but actually the change is this:

for (int i = 0; i < 2; i++) {
        for (p = sinfo; p != NULL; p = p->ai_next) {
            if (i == 0 ^ p->ai_family == family) {
                continue;
            }
codecov[bot] commented 2 years ago

Codecov Report

Merging #87 (2d957b4) into master (871e848) will increase coverage by 0.00%. The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #87   +/-   ##
=======================================
  Coverage   99.81%   99.81%           
=======================================
  Files          21       21           
  Lines        2188     2192    +4     
  Branches      383      385    +2     
=======================================
+ Hits         2184     2188    +4     
  Partials        4        4           
Impacted Files Coverage Δ
socket/sc_sock.c 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 871e848...2d957b4. Read the comment docs.

svladykin commented 2 years ago

With Hide whitespace enabled the diff looks better.

tezc commented 2 years ago

@svladykin Thanks, it perfectly makes sense.