Add option "-b" which allows to bind to non default address for outgoing connections

[cperciva]

Thanks for all the updates; I'll try to look at them tomorrow. @gperciva might be able to look at them more before I find time.

[nerijus]

There is a warning:

../libcperciva/network/network_connect.c: In function ‘network_connect_timeo’:
../libcperciva/network/network_connect.c:227:10: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
  C->sa_b = sa_b;
          ^

Should I remove const?

[gperciva]

In https://github.com/Tarsnap/spiped/pull/354/commits/f439f1ae4c1720e52f385d336c173d1d1b054fb6, I added a const to the definition of sa_b inside the struct, but I haven't looked at this PR in sufficient detail to judge if that was the correct fix or not.

I'll note that struct sock_addr * const * sas contains a const, so I'd be surprised if sa_b didn't have one somewhere.

[cperciva]

Need to add sock_addr_freelist(sas_b); in the non-error exit path of the two main functions.

[nerijus]

sock_addr_freelist(sas_b); in the non-error exit path of the two main functions Done.

[nerijus]

I've rebased my PR on the branch libcperciva-connect-bind (PR #354).

[nerijus]

Replying to https://github.com/Tarsnap/spiped/issues/351#issuecomment-1110190847

Good catch about the port number. We could probably add :0 to the bind address if it doesn't have a port number specified, to tell the OS to pick any port number it likes.

I should probably do this also; could someone please guide me where?

[nerijus]

I removed libcperciva/* changes from PR, as they are merged already.

[cperciva]

Replying to #351 (comment)

Good catch about the port number. We could probably add :0 to the bind address if it doesn't have a port number specified, to tell the OS to pick any port number it likes.

I should probably do this also; could someone please guide me where?

Sorry, I totally forgot about this. I would say in the two main functions:

1. Look for the final : character in the bind address (using strrchr),
2. Check that everything after that is a digit.

If there is no : or there is a non-digit character following it (as in the case of [ipv6::addr]) then use asprintf to construct a string with the :0 appended.

[cperciva]

While I'm thinking about bind addresses: Do we fail with a sane error message if someone tries to bind to a unix socket address and connect over TCP, or bind to an IP address and connect to a unix socket?

[gperciva]

Sorry, I totally forgot about this. I would say in the two main functions:

What would you think about having a separate function in main.c, instead of the code being inside the main()? I'm thinking of something like

static struct sock_addr **
required_sock_resolve(const char * addr, int add_zero_port)

that function would then be used for both opt_b and opt_t, and the same function.

... of course, that begs raises the question of whether it's worth adding such a function to sock.h, perhaps called sock_resolve_required().

[gperciva]

On the topic of a theoretical sock_resolve_required...

perftests/recv-zeros/main.c does:

if ((sas = sock_resolve(addr)) == NULL)
    goto err1;

if ((socket = sock_listener(sas[0])) == -1)
    goto err2;

However, sock_listener() doesn't check that its argument is not NULL; it jumps immediately to dereferencing sa->ai_family.

1) should sock_listener() check for a NULL, or is it a violation of the API to pass it a NULL?

2) I'm quite willing to believe that I screwed up when writing perftests/recv-zeros/main.c by not checking for sas[0] == NULL, but in which scenario would we want to have sock_resolve() return successfully with an empty list?

[nerijus]

Replying to #351 (comment)

1. Look for the final : character in the bind address (using strrchr),
2. Check that everything after that is a digit.

If there is no : or there is a non-digit character following it (as in the case of [ipv6::addr]) then use asprintf to construct a string with the :0 appended.

I pushed a commit which tries to implement it for spiped (will do similar for spipe if it is ok).

[nerijus]

While I'm thinking about bind addresses: Do we fail with a sane error message if someone tries to bind to a unix socket address and connect over TCP, or bind to an IP address and connect to a unix socket?

No, will have to do it too.

[nerijus]

FYI - I tested current code in production with both with and without -b option, it works ok, no memleaks or smth.

[gperciva]

Hi @nerijus,

I extracted the "add a port number if necessary" code into #358, and Colin added some review comments to it. Could you please continue the work on sock_addr_ensure_port()?

[nerijus]

OK, will do it a bit later.

[cperciva]

@nerijus ping?

[nerijus]

Sorry for the delay, rebased on upstream/sock_util_ensure_port and started using sock_addr_ensure_port().

[gperciva]

Hi @nerijus, I'll take care of Colin's concerns about sock_addr_ensure_port(), and any other libcperciva changes that might be required.

[gperciva]

Hi @nerijus, I'll take care of the rest of this PR, since the remaining stuff is all annoying nitpicky stuff. Thanks for all your work on this!

[gperciva]

Hi @nerijus, we've now merged this functionality in #364. Thanks for all your work on this!