> On Fri, Feb 08, 2008, Amos Jeffries wrote: > >> 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. > > How'd that screw things up? Hm, I'll have to read. > >> ... (1) pointer to somewhere holding the sockaddr_storage/*_in/*_in6 >> struct sockaddr *ai_addr; > > Which you have to allocate.
and do when needed. > >> char *ai_canonname; > > Which we're not using, and you have to allocate. and don't. > >> ... and because some system lookups in IPv6 can return _multiple_ >> addresses >> ... it has a linked-list of branchs for each of the alternatives >> struct addrinfo *ai_next; /* Pointer to next in list. */ >> }; >> > > Which is only useful in the context of providing lists of addresses for > a given host or IP address. What getaddrinfo() returns. and consider read-only. > >> > Why do we need AddrInfo now >> >> To kill dozens of castings, magic operations on object sizes, and cut >> down >> dozens of lines of 'compatibility code'. > > And add in a minimum of one, and a maximum of two allocations per socket > operation. > >> Together they make a pretty tree. But every used piece is eseentially >> another new, memset, free. > > Ah, and here you will have problems. Agreed. > > The members of that struct should probably be malloc, free, and not > new/delete. You're using new/delete which -should- map to a default > new operator and head off to the malloc libraries, but -squids- idea > of the malloc interface could differ from the -library- idea of > the malloc interface. I was thinking squid overiding the new/delete to its xmalloc/xfree at the same time it overrode the general malloc/free. > > So you should probably drop the new/delete'ing of the addrinfo stuff > and replace it with malloc/free. If thats better, no problem. > > You're also memset()'ing the addrinfo struct whether you allocated > it or not, which may be double-memset'ing the thing, and if someone > passed in an addrinfo it may have structure members which have now > been leaked. In the current usage the call should be the allocation. Not external. Allowing external allocation via another API call would be the only optimisation I can think of that does not break anything anywhere. memset is needed there because I could not tell that the new/delete _guaranteed_ pre-NULL'd memory and a single set bit in any unused fields might cause a crash later. With your deeper knowledge of the memory allocation in squid-3, feel free to alter the default memset()'s. Amos