Amos Jeffries wrote:
Alex Rousskov wrote:
On Fri, 2008-02-08 at 15:52 +1300, Amos Jeffries wrote:
The second;
sockaddr_storage (as Husni uses, and Adrian mentioned) was created to
provide a better way of using sockaddr* so the sockaddr_in and
sockaddr_in6 bits could be read-written easily. But the big/litte endian
problems between OS screwed up the sockaddr_in* sa_family field
locations
inside it so developers still can't portably use it for the v6/v6 flag
they wanted.
Do you have a reference for that? I do not want to bug you with more
questions but I am surprised to learn that some kind of a
sockaddr_storage wrapper cannot work well for Squid... We may have to
fix Polygraph that is using that approach, IIRC.
I don't have any online references. Everything I have found online
indicated that the _storage should be padded properly.
But, I found in my hex-level debugging of the IPv6 code in squid that
some things consistently went badly because the cast sa_family field was
something much higher than any IANA protocol family. Its too long ago
now to recall exact test cases. May have been a system-specific given
this connect() bug.
Correction: I did have online source that were somewhat confusing but
explained the behaviour a little. This seems to be saying what I
remember, http://www.kame.net/newsletter/19980604/
Amos
The third;
addrinfo* defines a whole new type. Wrapping that old sockaddr*
mess and
providing in a nice set fo bells and whistles for use. Most importantly
that flag we need to pas to the system calls.
I have to say that the "nice set of bells and whistles" in a basic
address structure used throughout a performance-sensitive program raises
red flags, but perhaps the actual performance implications are not as
bad.
The important bits for the squid comm code are:
struct addrinfo
{
int ai_family;
int ai_socktype;
int ai_protocol;
struct sockaddr *ai_addr; // pointer to sockaddr_storage/*_in/*_in6
int ai_addrlen;
char *ai_canonname; // we never new/free this ourselves in squid.
struct addrinfo *ai_next; // Pointer to next in list.
};
Can you replace IPAddress data members with the above, except not use
any pointers and forget about ai_next and ai_canonname? I think doing so
will eliminate temporary allocations and other things that look rather
scary to both code quality and performance folks.
In the cases where addrinfo* is going into read-only usage yes.
When you do need a list of addresses or canonname, it is OK to use
addrinfo and convert from/to IPAddress as needed, of course. I am
guessing such uses will not affect performance or overall code quality.
Amos
--
Please use Squid 2.6STABLE17+ or 3.0STABLE1+
There are serious security advisories out on all earlier releases.
