From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43489) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1alCU5-0007Xm-Ql for qemu-devel@nongnu.org; Wed, 30 Mar 2016 05:32:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1alCU2-0004Ph-Jc for qemu-devel@nongnu.org; Wed, 30 Mar 2016 05:32:17 -0400 Received: from mx1.redhat.com ([209.132.183.28]:42547) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1alCU2-0004Pc-BR for qemu-devel@nongnu.org; Wed, 30 Mar 2016 05:32:14 -0400 References: <1459208679-27805-1-git-send-email-samuel.thibault@ens-lyon.org> <1459208679-27805-4-git-send-email-samuel.thibault@ens-lyon.org> From: Thomas Huth Message-ID: <56FB9D1A.4000805@redhat.com> Date: Wed, 30 Mar 2016 11:32:10 +0200 MIME-Version: 1.0 In-Reply-To: <1459208679-27805-4-git-send-email-samuel.thibault@ens-lyon.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 3/5] slirp: Add dns6 resolution List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Samuel Thibault , qemu-devel@nongnu.org Cc: jan.kiszka@siemens.com, jasowang@redhat.com, armbru@redhat.com On 29.03.2016 01:44, Samuel Thibault wrote: > This makes get_dns_addr address family-agnostic, thus allowing to add t= he > IPv6 case. >=20 > Signed-off-by: Samuel Thibault > --- > slirp/ip6.h | 9 +++++++ > slirp/libslirp.h | 1 + > slirp/slirp.c | 72 ++++++++++++++++++++++++++++++++++++++++--------= -------- > slirp/socket.c | 4 ++-- > 4 files changed, 64 insertions(+), 22 deletions(-) [...] > diff --git a/slirp/slirp.c b/slirp/slirp.c > index 2f0d3a3..3558b47 100644 > --- a/slirp/slirp.c > +++ b/slirp/slirp.c > @@ -45,7 +45,9 @@ static QTAILQ_HEAD(slirp_instances, Slirp) slirp_inst= ances =3D > QTAILQ_HEAD_INITIALIZER(slirp_instances); > =20 > static struct in_addr dns_addr; > +static struct in6_addr dns6_addr; > static u_int dns_addr_time; > +static u_int dns6_addr_time; > =20 > #define TIMEOUT_FAST 2 /* milliseconds */ > #define TIMEOUT_SLOW 499 /* milliseconds */ > @@ -99,6 +101,11 @@ int get_dns_addr(struct in_addr *pdns_addr) > return 0; > } > =20 > +int get_dns6_addr(struct in6_addr *pdns_addr6) > +{ > + return -1; > +} > + > static void winsock_cleanup(void) > { > WSACleanup(); > @@ -106,36 +113,40 @@ static void winsock_cleanup(void) > =20 > #else > =20 > -static struct stat dns_addr_stat; > - > -static int get_dns_addr_cached(struct in_addr *pdns_addr) > +static int get_dns_addr_cached(void *pdns_addr, void *cached_addr, > + socklen_t addrlen, > + struct stat *cached_stat, u_int *cached= _time) > { > struct stat old_stat; > if ((curtime - dns_addr_time) < TIMEOUT_DEFAULT) { Shouldn't that be "*cached_time" now instead of "dns_addr_time"? Or what is this cached_time parameter now good for? > - *pdns_addr =3D dns_addr; > + memcpy(pdns_addr, cached_addr, addrlen); > return 0; > } > - old_stat =3D dns_addr_stat; > - if (stat("/etc/resolv.conf", &dns_addr_stat) !=3D 0) { > + old_stat =3D *cached_stat; > + if (stat("/etc/resolv.conf", cached_stat) !=3D 0) { > return -1; > } > - if ((dns_addr_stat.st_dev =3D=3D old_stat.st_dev) > - && (dns_addr_stat.st_ino =3D=3D old_stat.st_ino) > - && (dns_addr_stat.st_size =3D=3D old_stat.st_size) > - && (dns_addr_stat.st_mtime =3D=3D old_stat.st_mtime)) { > - *pdns_addr =3D dns_addr; > + if ((cached_stat->st_dev =3D=3D old_stat.st_dev) > + && (cached_stat->st_ino =3D=3D old_stat.st_ino) > + && (cached_stat->st_size =3D=3D old_stat.st_size) > + && (cached_stat->st_mtime =3D=3D old_stat.st_mtime)) { > + memcpy(pdns_addr, cached_addr, addrlen); > return 0; > } > return 1; > } > =20 > -static int get_dns_addr_resolv_conf(struct in_addr *pdns_addr) > +static int get_dns_addr_resolv_conf(int af, void *pdns_addr, void *cac= hed_addr, > + socklen_t addrlen, u_int *cached_t= ime) > { > char buff[512]; > char buff2[257]; > FILE *f; > int found =3D 0; > - struct in_addr tmp_addr; > + void *tmp_addr =3D alloca(addrlen); > +#ifdef DEBUG > + char s[INET6_ADDRSTRLEN]; > +#endif > =20 > f =3D fopen("/etc/resolv.conf", "r"); > if (!f) > @@ -146,13 +157,14 @@ static int get_dns_addr_resolv_conf(struct in_add= r *pdns_addr) > #endif > while (fgets(buff, 512, f) !=3D NULL) { > if (sscanf(buff, "nameserver%*[ \t]%256s", buff2) =3D=3D 1) { > - if (!inet_aton(buff2, &tmp_addr)) > + if (!inet_pton(af, buff2, tmp_addr)) { > continue; > + } > /* If it's the first one, set it to dns_addr */ > if (!found) { > - *pdns_addr =3D tmp_addr; > - dns_addr =3D tmp_addr; > - dns_addr_time =3D curtime; > + memcpy(pdns_addr, tmp_addr, addrlen); > + memcpy(cached_addr, tmp_addr, addrlen); > + *cached_time =3D curtime; > } > #ifdef DEBUG > else > @@ -166,7 +178,7 @@ static int get_dns_addr_resolv_conf(struct in_addr = *pdns_addr) > } > #ifdef DEBUG > else Maybe you could also add proper curly braces for this else-path now, and then add the "char s[INET6_ADDRSTRLEN]" definition here instead ... that way you don't need the separate "#ifdef DEBUG" at the beginning of this function. > - fprintf(stderr, "%s", inet_ntoa(tmp_addr)); > + fprintf(stderr, "%s", inet_ntop(af, tmp_addr, s, sizeo= f(s))); While inet_ntoa never returns NULL, inet_ntop might return NULL on errors. I think glibc fprintf will likely survive such a NULL pointer, but it might still be better to check the return value and print something else if the return value was NULL ? > #endif > } > } > @@ -176,16 +188,36 @@ static int get_dns_addr_resolv_conf(struct in_add= r *pdns_addr) > return 0; > } > =20 > +static struct stat dns_addr_stat; You could also move this definition into the get_dns_addr() function. > int get_dns_addr(struct in_addr *pdns_addr) > { > if (dns_addr.s_addr !=3D 0) { > int ret; > - ret =3D get_dns_addr_cached(pdns_addr); > + ret =3D get_dns_addr_cached(pdns_addr, &dns_addr, sizeof(dns_a= ddr), > + &dns_addr_stat, &dns_addr_time); > + if (ret <=3D 0) { > + return ret; > + } > + } > + return get_dns_addr_resolv_conf(AF_INET, pdns_addr, &dns_addr, > + sizeof(dns_addr), &dns_addr_time); > +} > + > +static struct stat dns6_addr_stat; You could also move this definition into the get_dns6_addr() function. > +int get_dns6_addr(struct in6_addr *pdns6_addr) > +{ > + if (!in6_zero(&dns6_addr)) { > + int ret; > + ret =3D get_dns_addr_cached(pdns6_addr, &dns6_addr, sizeof(dns= 6_addr), > + &dns6_addr_stat, &dns6_addr_time); > if (ret <=3D 0) { > return ret; > } > } > - return get_dns_addr_resolv_conf(pdns_addr); > + return get_dns_addr_resolv_conf(AF_INET6, pdns6_addr, &dns6_addr, > + sizeof(dns6_addr), &dns6_addr_time= ); > } Thomas