Re: Ip::Address::IsAnyAddr

On 07/29/2011 05:22 PM, Amos Jeffries wrote:
> On 30/07/11 03:50, Alex Rousskov wrote:
>> On 07/25/2011 08:59 AM, Tsantilas Christos wrote:
>>> I am sending a version 4 of the patch which is the same with Amos patch
>>> but is a little smaller. IT is easier to see the changes.
>>
>>
>>> bool
>>> Ip::Address::IsIPv4() const
>>> {
>>> - return IsAnyAddr() || IsNoAddr() ||
>>> IN6_IS_ADDR_V4MAPPED(&m_SocketAddr.sin6_addr );
>>> + return IN6_IS_ADDR_V4MAPPED(&m_SocketAddr.sin6_addr );
>>> }
>>>
>>> bool
>>> Ip::Address::IsIPv6() const
>>> {
>>> - return IsAnyAddr() || IsNoAddr() ||
>>> !IN6_IS_ADDR_V4MAPPED(&m_SocketAddr.sin6_addr );
>>> + return !IN6_IS_ADDR_V4MAPPED(&m_SocketAddr.sin6_addr );
>>> }
>>
>> Can we rewrite IsIPv6() as "return !IsIPv4()", to clarify the intent if
>> that is indeed the intent?
>
> Internally maybe. How long until the unix socket address gets stored
> there too and breaks that?

When the unix socket address gets stored there too, the patch code will
similarly break, so there is no difference from that point of view.

When we have three address kinds, they would still be mutually
exclusive, and the updated code would have to reflect that somehow so
the change I propose would be a step in the right direction compared to
the current or patch code. It simply tells us "it is either this or that".

>> Also, the documentation for these two methods seems to imply a different
>> relationship, at least in some corner cases:
>>
>>> /** Test whether content can be used as an IPv4 address
>>> \retval true if content was received as an IPv4 address
>>> \retval true if content was received as an IPv4-Mapped address
>>> \retval false if content was received as a non-mapped IPv6
>>> native address.
>>> */
>>> bool IsIPv4() const;
>>>
>>> /** Test whether content can be used as an IPv6 address.
>>> \retval true if --enable-ipv6 has been compiled.
>>> \retval false if --disable-ipv6 has been compiled.
>>> \retval false if --with-ipv6-split-stack has been compiled AND
>>> content is I
>>> Pv4-mapped.
>>> */
>>> bool IsIPv6() const;
>>
>> As far as I can tell, the above definitions make it possible for both
>> IsIPv4() and IsIPv6() to return true at the same time in some cases, but
>> the implementation does not support that. Thus, the docs or the
>> implementation is wrong.
>
> The docs have become wrong. Since we can now convert one to the other in
> some cases. That IPv4() should say the content is currenty an IPv4-only
> address except for no-addr which may be both simultaneously still.
> IPv6() should say its an IP address outside the IPv4-mapped range or
> IPv4 no-addr.
>
> Something like that anyway.

IsIPv?() implementations in the patch do not support the notion of an
address being simultaneously IPv4 and IPv6. Whether that is wrong is a
separate question. Based on your response, it sounds like the current
documentation is wrong, but it is not clear (to me) what it should be
because it is not clear whether you consider the "mutually exclusive"
code in the patch correct.

Thank you,

Alex.
Received on Fri Jul 29 2011 - 23:57:06 MDT