Subject: Re: [PATCH] Allow the use of IPv6 nameservers

Re: [PATCH] Allow the use of IPv6 nameservers

From: Yang Tse <yangsita_at_gmail.com>
Date: Thu, 18 Feb 2010 16:59:13 +0100

2010/2/12, Jakub Hrozek wrote:

> A new patch is attached that also fixes ares_dup() and adds the manpage.
>
> Apart from the high-level summary in the patch itself, here's the changes:
>
> * struct "server state" no longer holds the address in "struct
> in_addr" but rather "struct addrinfo *"
> * when initializing from resolv.conf, getaddrinfo() is used in place
> of inet_addr()
> * a new API call ares_set_nameserves which can set the nameservers used
> * in order not to break "struct ares_options", ares_save_options and
> ares_init_options still works with v4 nameservers only. ares_dup() works
> for both v4 and v6 servers.
>
> More info is, of course, in the patch :-)

Ok, some comments on an initial review...

Makefile.inc is missing ares_set_nameservers.html and ares_set_nameservers.pdf.

ares_set_nameservers.3 first line has ".TH
ARES_SET_SOCKET_CALLBACK..." it should be changed to ".TH
ARES_SET_NAMESERVERS...".

And now some real meat which affects the whole patch...

I believe that changing server_state struct 'addr' member data type to
an 'ares_addr struct' instead than a pointer to 'struct addrinfo'
serves the same purposes while keeping things a bit simpler later on,
avoiding the need for free_addr member in server_state struct. Unless
I've overlooked something.

I also believe that moving private definition of the ares_addr struct
from ares_private.h to the external interface in file ares.h making it
publicly available would also help to make the interface to
ares_set_nameservers() more palatable.

CARES_EXTERN int ares_set_nameservers(ares_channel channel,
                                      struct ares_addr *server_addr,
                                      int num_servers);

Another important thing...

Not all systems have getaddrinfo(), so if you want to use it you have
to protect its usage with preprocessor symbol HAVE_GETADDRINFO,
definition which fortunately is already available in config.h for
systems that have it.

Another twist with getaddrinfo(), for portability reasons, don't store
addrinfo structures returned by getaddrinfo() directly, nor pointers
to these. Copy the contents you need from the returned addrinfo
structs and call freeaddrinfo() ASAP.

-- 
-=[Yang]=-
Received on 2010-02-18