Mauro Tortonesi <[EMAIL PROTECTED]> writes:

> and i'm saying that for this task the ideal structure is
> sockaddr_storage. notice that my code uses sockaddr_storage
> (typedef'd as wget_sockaddr) only when dealing with socket
> addresses, not for ip address caching.

Now I see.  Thanks for clearing it up.

>> An IPv4 address is nothing more than a 32-bit quantity.  I don't
>> see anything "incorrect" about using unsigned char[4] for that, and
>> that works perfectly fine on 64-bit architectures.
>
> ok, but in this way you have to call memcmp each time you want to compare
> two ip addresses and memcpy each time you want to copy an ip
> address.

Well, you can copy addresses with the assignment operator as well, as
long as they're in a `struct', as they are in the current code.  You
do need `memcmp' to compare them, but that's fine with me.

> i prefer the in_addr approach (and i don't understand why we
> shouldn't use structures like in_addr and in_addr6 which have been
> created just to do what we want: storing ip addresses)

Because they're complexly defined and hard to read if all you want is
to store 4 and 16 bytes of binary data, respectively.

> however, notice that using unsigned char[4] and unsigned char[16] is
> a less portable solution and is potentially prone to problems with
> the alignement of the sockaddr_in and sockaddr_in6 structs.

Note that I only propose using unsigned char[N] for internal storing
of addresses, such as in Wget's own `struct ip_address'.  For talking
to system API's, we can and should copy the address to the appropriate
sockaddr_* structure.  That's how the current code works, and it's
quite portable.

>> Besides, you seem to be willing to cache the string representation
>> of an IP address.  Why is it acceptable to work with a char *, but
>> unacceptable to work with unsigned char[4]?  I simply don't see
>> that in_addr is helping anything in host.c's code base.
>
> i would prefer to cache string representation of ip addresses
> because the ipv6 code would be much simpler and more elegant.

I agree.  My point was merely to point out that even you yourself
believe that struct in_addr* is not the only legitimate way to store
an IP address.

>> >> I don't understand the new PASSIVE flag to lookup_host.
>> >
>> > well, that's a problem. to get a socket address suitable for
>> > bind(2), you must call getaddrinfo with the AI_PASSIVE flag set.
>>
>> Why?  The current code seems to get by without it.
>
> the problem is when you call lookup_host to get a struct to pass to
> bind(2). if you use --bind-address=localhost and you don't set the
> AI_PASSIVE flag, getaddinfo will return the 127.0.0.1 address, which
> is incorrect.
>
>> There must be a way to get at the socket address without calling
>> getaddrinfo.
>
> not if you want to to use --bind-address=ipv6only.domain.com.

I see.  I guess we'll have to live with it, one way or the other.
Instead of accumulating boolean arguments, lookup_host should probably
accept a FLAGS argument, so you can call it with, e.g.:

    lst = lookup_host (addr, LH_PASSIVE | LH_SILENT);
    ...

Reply via email to