[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/5] slirp: Add dns6 resolution
From: |
Thomas Huth |
Subject: |
Re: [Qemu-devel] [PATCH 3/5] slirp: Add dns6 resolution |
Date: |
Wed, 30 Mar 2016 11:32:10 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.0 |
On 29.03.2016 01:44, Samuel Thibault wrote:
> This makes get_dns_addr address family-agnostic, thus allowing to add the
> IPv6 case.
>
> Signed-off-by: Samuel Thibault <address@hidden>
> ---
> 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_instances =
> QTAILQ_HEAD_INITIALIZER(slirp_instances);
>
> static struct in_addr dns_addr;
> +static struct in6_addr dns6_addr;
> static u_int dns_addr_time;
> +static u_int dns6_addr_time;
>
> #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;
> }
>
> +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)
>
> #else
>
> -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 = dns_addr;
> + memcpy(pdns_addr, cached_addr, addrlen);
> return 0;
> }
> - old_stat = dns_addr_stat;
> - if (stat("/etc/resolv.conf", &dns_addr_stat) != 0) {
> + old_stat = *cached_stat;
> + if (stat("/etc/resolv.conf", cached_stat) != 0) {
> return -1;
> }
> - if ((dns_addr_stat.st_dev == old_stat.st_dev)
> - && (dns_addr_stat.st_ino == old_stat.st_ino)
> - && (dns_addr_stat.st_size == old_stat.st_size)
> - && (dns_addr_stat.st_mtime == old_stat.st_mtime)) {
> - *pdns_addr = dns_addr;
> + if ((cached_stat->st_dev == old_stat.st_dev)
> + && (cached_stat->st_ino == old_stat.st_ino)
> + && (cached_stat->st_size == old_stat.st_size)
> + && (cached_stat->st_mtime == old_stat.st_mtime)) {
> + memcpy(pdns_addr, cached_addr, addrlen);
> return 0;
> }
> return 1;
> }
>
> -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
> *cached_addr,
> + socklen_t addrlen, u_int *cached_time)
> {
> char buff[512];
> char buff2[257];
> FILE *f;
> int found = 0;
> - struct in_addr tmp_addr;
> + void *tmp_addr = alloca(addrlen);
> +#ifdef DEBUG
> + char s[INET6_ADDRSTRLEN];
> +#endif
>
> f = fopen("/etc/resolv.conf", "r");
> if (!f)
> @@ -146,13 +157,14 @@ static int get_dns_addr_resolv_conf(struct in_addr
> *pdns_addr)
> #endif
> while (fgets(buff, 512, f) != NULL) {
> if (sscanf(buff, "nameserver%*[ \t]%256s", buff2) == 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 = tmp_addr;
> - dns_addr = tmp_addr;
> - dns_addr_time = curtime;
> + memcpy(pdns_addr, tmp_addr, addrlen);
> + memcpy(cached_addr, tmp_addr, addrlen);
> + *cached_time = 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, sizeof(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_addr
> *pdns_addr)
> return 0;
> }
>
> +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 != 0) {
> int ret;
> - ret = get_dns_addr_cached(pdns_addr);
> + ret = get_dns_addr_cached(pdns_addr, &dns_addr, sizeof(dns_addr),
> + &dns_addr_stat, &dns_addr_time);
> + if (ret <= 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 = get_dns_addr_cached(pdns6_addr, &dns6_addr, sizeof(dns6_addr),
> + &dns6_addr_stat, &dns6_addr_time);
> if (ret <= 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
- [Qemu-devel] [PATCH 0/5] ipv4-only and ipv6-only support, Samuel Thibault, 2016/03/28
- [Qemu-devel] [PATCH 5/5] slirp: Add RDNSS advertisement, Samuel Thibault, 2016/03/28
- [Qemu-devel] [PATCH 1/5] slirp: Allow disabling IPv4 or IPv6, Samuel Thibault, 2016/03/28
- [Qemu-devel] [PATCH 2/5] slirp: Split get_dns_addr, Samuel Thibault, 2016/03/28
- [Qemu-devel] [PATCH 3/5] slirp: Add dns6 resolution, Samuel Thibault, 2016/03/28
- Re: [Qemu-devel] [PATCH 3/5] slirp: Add dns6 resolution,
Thomas Huth <=
- [Qemu-devel] [PATCH 4/5] slirp: Support link-local DNS addresses, Samuel Thibault, 2016/03/28