Re: squid-3.HEAD IPAddress leak

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.

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

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.

Thank you,

Alex.
Received on Thu Feb 07 2008 - 21:07:35 MST