On Wed, 8 Oct 2003, Hrvoje Niksic wrote:

> Mauro Tortonesi <[EMAIL PROTECTED]> writes:
>
> >> I still don't understand the choice to use sockaddr and
> >> sockaddr_storage in a application code.
> >> They result in needless casts and (to me) uncomprehensible code.
> >
> > well, using sockaddr_storage is the right way (TM) to write IPv6 enabled
> > code ;-)
>
> Not when the only thing you need is storing the result of a DNS
> lookup.

i agree, in fact for that case we use the ip_address struct.


> I've seen the RFC, but I don't agree with it in the case of Wget.  In
> fact, even the RFC states that the data structure is merely a help for
> writing portable code across "multiple address families and
> platforms".  Wget doesn't aim for AF independence, and the
> alternatives are at least as good for platform independence.

i think we are talking about two different things. i am talking about
storing generic socket addresses (IPv4 or IPv6) 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. and i am not trying to write
af-indipendent code, anyway.


> >> For example, this cast: (unsigned char *)(&addr->addr_v4.s_addr)
> >> would not be necessary if the address were defined as unsigned
> >> char[4].
> >
> > in_addr is the correct structure to store ipv4 addresses. using
> > in_addr instead of unsigned char[4] makes much easier to copy or
> > compare ipv4 addresses. moreover, you don't have to care about the
> > integer size in 64-bits architectures.
>
> 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.
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) as i think it is a more elegant
solution, but if you still wish to use unsigned char[4] and unsigned
char[16] i cannot force you to change your mind.

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.


> 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 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.


> >> If it would make your life easier to add TYPE in !ENABLE_IPV6 case,
> >> so you can write it more compactly, by all means do it.  By "more
> >> compactly" I mean something code like this:
> >>
> [...]
> > that's a question i was going to ask you. i supposed you were
> > against adding the type member to ip_address in the IPv4-only case,
>
> Maintainability is more important than saving a few bytes per cached
> IP address, especially since I don't expect the number of cache
> entries to ever be large enough to make a difference.  (If someone
> downloads from so many addresses that the hash table sizes become a
> problem, the TYPE member will be the least of his problems.)

i agree.


> > P.S. please notice that by caching the string representation of IP
> >      addresses instead of their network representation, the code
> >      could become much more elegant and simple.
>
> You said that before, but I don't quite understand why that's the
> case.  It's certainly not the case for IPv4.

sure, but that would be of great help, especially for IPv6. the code (and
in particular the translation from ip addresses to socket addresses and
viceversa) would be much simpler and elegant.


-- 
Aequam memento rebus in arduis servare mentem...

Mauro Tortonesi                 [EMAIL PROTECTED]
                                [EMAIL PROTECTED]
                                [EMAIL PROTECTED]
Deep Space 6 - IPv6 with Linux  http://www.deepspace6.net
Ferrara Linux User Group        http://www.ferrara.linux.it


Reply via email to