tr_session::publicAddress seems wrong for IPv4

[lvella]

What is the issue?

The relevant code is:

    if (type == TR_AF_INET)
    {
        // if user provided an address, use it.
        // otherwise, use any_ipv4 (0.0.0.0).
        return global_ip_cache_->bind_addr(type);
    }

Sorry if this is a false alarm, but unless the term "public address" is used though the code differently from the common accepted meaning of public address, due to NAT, bind address is almost never the public address, much less 0.0.0.0.

Which application of Transmission?

transmission-daemon

Which version of Transmission?

52b03c2

[tearfur]

It seems to me that it is just the naming of the function that's strange. If you look the usages of tr_session::publicAddress(), its return value is mostly just used as the bind address.

@ckerr However, I do think that it got confused as the actual public address of the client here:

transmission/libtransmission/peer-msgs.cc

Lines 961 to 965 in 8747267

	if (auto const addr = msgs->session->publicAddress(TR_AF_INET6); !addr.is_any())
	{
	TR_ASSERT(addr.is_ipv6());
	tr_variantDictAddRaw(&val, TR_KEY_ipv6, &addr.addr.addr6, sizeof(addr.addr.addr6));
	}

It looks like it is used as the ipv6 key of a BEP-10 handshake?

[ThePowerTool]

In this instance 0.0.0.0 is stated as a mask meaning "any address", as stated.

You specifically stated a concern with the use of "public address" but I don't see "public address" in the code you referenced, above.

[lvella]

You specifically stated a concern with the use of "public address" but I don't see "public address" in the code you referenced, above.

publicAddress is the name of the function containing that code

[ThePowerTool]

the term "public address" is used though the code differently

I did take a quick look at the code (before that post as you provided a handy pointer to the specific line) and saw the function publicAddress(). I didn't attribute the function to your comment as you specified "term" instead of "function." I'm very literal--and sometimes I screw that up.

As stated, it appears to be generically referring to the existing IP address on which the client resides. This is important because there are scenarios where you would specify a different IP address (hence, the publicAddress() function).

:-)

[tearfur]

Especially now that we have tr_session::global_address(), publicAddress() should probably be renamed to bind_address() to avoid confusion.

[tearfur]

Especially now that we have tr_session::global_address(), publicAddress() should probably be renamed to bind_address() to avoid confusion.

Renamed in #5644.

Edit: Should be #5643. :P

[ckerr]

Did 5644 resolve this ticket? (TBH I think I might be misunderstanding the request and have been standing back since @tearfur seems to be doing great on these issues)