From 9035325e566c47e83700f88b1ff6376dce0068e8 Mon Sep 17 00:00:00 2001 From: Roman Yepishev Date: Sat, 27 Feb 2016 11:45:02 -0500 Subject: [PATCH] Remove magic numbers from addr_resolve * Add #defines for INET/INET6 returns * Remove magic number 3 - exact AF_INET/INET6 result found. * Updated network_test.c --- auto_tests/network_test.c | 14 +++++++------- toxcore/network.c | 37 +++++++++++++++++++++---------------- toxcore/network.h | 4 ++++ 3 files changed, 32 insertions(+), 23 deletions(-) diff --git a/auto_tests/network_test.c b/auto_tests/network_test.c index c1ac8084..adf0189a 100644 --- a/auto_tests/network_test.c +++ b/auto_tests/network_test.c @@ -28,7 +28,7 @@ START_TEST(test_addr_resolv_localhost) int localhost_split = 0; IP ip; - ip_init(&ip, 0); + ip_init(&ip, /* ipv6? */ 0); int res = addr_resolve(localhost, &ip, NULL); @@ -39,10 +39,10 @@ START_TEST(test_addr_resolv_localhost) ck_assert_msg(ip.ip4.uint32 == htonl(0x7F000001), "Expected 127.0.0.1, got %s.", inet_ntoa(ip.ip4.in_addr)); } - ip_init(&ip, 1); + ip_init(&ip, /* ipv6? */ 1); res = addr_resolve(localhost, &ip, NULL); - if (res < 1) { + if (!(res & TOX_ADDR_RESOLVE_INET6)) { res = addr_resolve("ip6-localhost", &ip, NULL); localhost_split = 1; } @@ -50,12 +50,12 @@ START_TEST(test_addr_resolv_localhost) ck_assert_msg(res > 0, "Resolver failed: %u, %s (%x, %x)", errno, strerror(errno)); if (res > 0) { - ck_assert_msg(ip.family == AF_INET6, "Expected family AF_INET6, got %u.", ip.family); + ck_assert_msg(ip.family == AF_INET6, "Expected family AF_INET6 (%u), got %u.", AF_INET6, ip.family); ck_assert_msg(!memcmp(&ip.ip6, &in6addr_loopback, sizeof(IP6)), "Expected ::1, got %s.", ip_ntoa(&ip)); } if (!localhost_split) { - ip_init(&ip, 1); + ip_init(&ip, /* ipv6? */ 1); ip.family = AF_UNSPEC; IP extra; ip_reset(&extra); @@ -63,10 +63,10 @@ START_TEST(test_addr_resolv_localhost) ck_assert_msg(res > 0, "Resolver failed: %u, %s (%x, %x)", errno, strerror(errno)); if (res > 0) { - ck_assert_msg(ip.family == AF_INET6, "Expected family AF_INET6, got %u.", ip.family); + ck_assert_msg(ip.family == AF_INET6, "Expected family AF_INET6 (%u), got %u.", AF_INET6, ip.family); ck_assert_msg(!memcmp(&ip.ip6, &in6addr_loopback, sizeof(IP6)), "Expected ::1, got %s.", ip_ntoa(&ip)); - ck_assert_msg(extra.family == AF_INET, "Expected family AF_INET, got %u.", extra.family); + ck_assert_msg(extra.family == AF_INET, "Expected family AF_INET (%u), got %u.", AF_INET, extra.family); ck_assert_msg(extra.ip4.uint32 == htonl(0x7F000001), "Expected 127.0.0.1, got %s.", inet_ntoa(extra.ip4.in_addr)); } } else { diff --git a/toxcore/network.c b/toxcore/network.c index 04efd03b..5ab480b7 100644 --- a/toxcore/network.c +++ b/toxcore/network.c @@ -939,7 +939,7 @@ int addr_parse_ip(const char *address, IP *to) * returns in *to a valid IPAny (v4/v6), * prefers v6 if ip.family was AF_UNSPEC and both available * returns in *extra an IPv4 address, if family was AF_UNSPEC and *to is AF_INET6 - * returns 0 on failure + * returns 0 on failure, TOX_ADDR_RESOLVE_* on success. */ int addr_resolve(const char *address, IP *to, IP *extra) { @@ -951,7 +951,9 @@ int addr_resolve(const char *address, IP *to, IP *extra) struct addrinfo *server = NULL; struct addrinfo *walker = NULL; struct addrinfo hints; - int rc; + int rc; + int result = 0; + int done = 0; memset(&hints, 0, sizeof(hints)); hints.ai_family = family; @@ -972,17 +974,18 @@ int addr_resolve(const char *address, IP *to, IP *extra) IP ip6; ip_init(&ip6, /* ipv6? */ true); - for (walker = server; (walker != NULL) && (rc != 3); walker = walker->ai_next) { + for (walker = server; (walker != NULL) && !done; walker = walker->ai_next) { switch (walker->ai_family) { case AF_INET: if (walker->ai_family == family) { /* AF_INET requested, done */ struct sockaddr_in *addr = (struct sockaddr_in *)walker->ai_addr; to->ip4.in_addr = addr->sin_addr; - rc = 3; // TODO do we really have to reuse variable instead of creating a new one? - } else if (!(rc & 1)) { /* AF_UNSPEC requested, store away */ + result = TOX_ADDR_RESOLVE_INET; + done = 1; + } else if (!(result & TOX_ADDR_RESOLVE_INET)) { /* AF_UNSPEC requested, store away */ struct sockaddr_in *addr = (struct sockaddr_in *)walker->ai_addr; ip4.ip4.in_addr = addr->sin_addr; - rc |= 1; // FIXME magic number + result |= TOX_ADDR_RESOLVE_INET; } break; /* switch */ @@ -992,13 +995,14 @@ int addr_resolve(const char *address, IP *to, IP *extra) if (walker->ai_addrlen == sizeof(struct sockaddr_in6)) { struct sockaddr_in6 *addr = (struct sockaddr_in6 *)walker->ai_addr; to->ip6.in6_addr = addr->sin6_addr; - rc = 3; + result = TOX_ADDR_RESOLVE_INET6; + done = 1; } - } else if (!(rc & 2)) { /* AF_UNSPEC requested, store away */ + } else if (!(result & TOX_ADDR_RESOLVE_INET6)) { /* AF_UNSPEC requested, store away */ if (walker->ai_addrlen == sizeof(struct sockaddr_in6)) { struct sockaddr_in6 *addr = (struct sockaddr_in6 *)walker->ai_addr; ip6.ip6.in6_addr = addr->sin6_addr; - rc |= 2; + result |= TOX_ADDR_RESOLVE_INET6; } } @@ -1006,21 +1010,22 @@ int addr_resolve(const char *address, IP *to, IP *extra) } } - if (to->family == AF_UNSPEC) { - if (rc & 2) { // FIXME magic number + if (family == AF_UNSPEC) { + if (result & TOX_ADDR_RESOLVE_INET6) { ip_copy(to, &ip6); - if ((rc & 1) && (extra != NULL)) { + if ((result & TOX_ADDR_RESOLVE_INET) && (extra != NULL)) { ip_copy(extra, &ip4); } - } else if (rc & 1) { + } else if (result & TOX_ADDR_RESOLVE_INET) { ip_copy(to, &ip4); - } else - rc = 0; + } else { + result = 0; + } } freeaddrinfo(server); - return rc; + return result; } /* diff --git a/toxcore/network.h b/toxcore/network.h index 56494562..8d2ccfce 100644 --- a/toxcore/network.h +++ b/toxcore/network.h @@ -176,6 +176,10 @@ IP_Port; #define TOX_ENABLE_IPV6_DEFAULT 1 +/* addr_resolve return values */ +#define TOX_ADDR_RESOLVE_INET 1 +#define TOX_ADDR_RESOLVE_INET6 2 + /* ip_ntoa * converts ip into a string * uses a static buffer, so mustn't used multiple times in the same output