Problem: address parsing code is tied to the TCP code

Solution: Factor the code into a different file with a well defined API and add
unit tests.

CMakeLists.txt

@@ -603,6 +603,7 @@ set (cxx-sources
         udp_address.cpp
         scatter.cpp
         gather.cpp
+	ip_resolver.cpp
 		zap_client.cpp
 		# at least for VS, the header files must also be listed
 		address.hpp

Makefile.am

@@ -83,6 +83,8 @@ src_libzmq_la_SOURCES = \
 	src/io_thread.hpp \
 	src/ip.cpp \
 	src/ip.hpp \
+	src/ip_resolver.cpp \
+	src/ip_resolver.hpp \
 	src/ipc_address.cpp \
 	src/ipc_address.hpp \
 	src/ipc_connecter.cpp \
@@ -895,7 +897,8 @@ if ENABLE_STATIC
 test_apps += \
 	unittests/unittest_poller \
 	unittests/unittest_ypipe \
-	unittests/unittest_mtrie
+	unittests/unittest_mtrie \
+	unittests/unittest_ip_resolver
 
 unittests_unittest_poller_SOURCES = unittests/unittest_poller.cpp
 unittests_unittest_poller_CPPFLAGS = -I$(top_srcdir)/src ${UNITY_CPPFLAGS} $(CODE_COVERAGE_CPPFLAGS)
@@ -920,6 +923,14 @@ unittests_unittest_mtrie_LDADD = $(top_builddir)/src/.libs/libzmq.a \
 	${src_libzmq_la_LIBADD} \
 	${UNITY_LIBS} \
 	$(CODE_COVERAGE_LDFLAGS)
+
+unittests_unittest_ip_resolver_SOURCES = unittests/unittest_ip_resolver.cpp
+unittests_unittest_ip_resolver_CPPFLAGS = -I$(top_srcdir)/src ${UNITY_CPPFLAGS} $(CODE_COVERAGE_CPPFLAGS)
+unittests_unittest_ip_resolver_CXXFLAGS = $(CODE_COVERAGE_CXXFLAGS)
+unittests_unittest_ip_resolver_LDADD = $(top_builddir)/src/.libs/libzmq.a \
+	${src_libzmq_la_LIBADD} \
+	${UNITY_LIBS} \
+	$(CODE_COVERAGE_LDFLAGS)
 endif
 
 check_PROGRAMS = ${test_apps}

src/ip_resolver.hpp

+/*
+    Copyright (c) 2007-2018 Contributors as noted in the AUTHORS file
+
+    This file is part of libzmq, the ZeroMQ core engine in C++.
+
+    libzmq is free software; you can redistribute it and/or modify it under
+    the terms of the GNU Lesser General Public License (LGPL) as published
+    by the Free Software Foundation; either version 3 of the License, or
+    (at your option) any later version.
+
+    As a special exception, the Contributors give you permission to link
+    this library with independent modules to produce an executable,
+    regardless of the license terms of these independent modules, and to
+    copy and distribute the resulting executable under terms of your choice,
+    provided that you also meet, for each linked independent module, the
+    terms and conditions of the license of that module. An independent
+    module is a module which is not derived from or based on this library.
+    If you modify this library, you must extend this exception to your
+    version of the library.
+
+    libzmq is distributed in the hope that it will be useful, but WITHOUT
+    ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+    FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public
+    License for more details.
+
+    You should have received a copy of the GNU Lesser General Public License
+    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#ifndef __ZMQ_IP_RESOLVER_HPP_INCLUDED__
+#define __ZMQ_IP_RESOLVER_HPP_INCLUDED__
+
+#if !defined ZMQ_HAVE_WINDOWS
+#include <sys/socket.h>
+#include <netinet/in.h>
+#endif
+
+namespace zmq
+{
+union ip_addr_t
+{
+    sockaddr generic;
+    sockaddr_in ipv4;
+    sockaddr_in6 ipv6;
+};
+
+class ip_resolver_options_t
+{
+  public:
+    ip_resolver_options_t ();
+
+    ip_resolver_options_t &bindable (bool bindable_);
+    ip_resolver_options_t &allow_nic_name (bool allow_);
+    ip_resolver_options_t &ipv6 (bool ipv6_);
+    ip_resolver_options_t &expect_port (bool expect_);
+    ip_resolver_options_t &allow_dns (bool allow_);
+
+    bool bindable ();
+    bool allow_nic_name ();
+    bool ipv6 ();
+    bool expect_port ();
+    bool allow_dns ();
+
+  private:
+    bool bindable_wanted;
+    bool nic_name_allowed;
+    bool ipv6_wanted;
+    bool port_expected;
+    bool dns_allowed;
+};
+
+class ip_resolver_t
+{
+  public:
+    ip_resolver_t (ip_resolver_options_t opts_);
+
+    int resolve (ip_addr_t *ip_addr_, const char *name_);
+
+  protected:
+    ip_resolver_options_t options;
+
+    int resolve_nic_name (ip_addr_t *ip_addr_, const char *nic_);
+    int resolve_getaddrinfo (ip_addr_t *ip_addr_, const char *addr_);
+
+#if defined ZMQ_HAVE_WINDOWS
+    int get_interface_name (unsigned long index, char **dest) const;
+    int wchar_to_utf8 (const WCHAR *src, char **dest) const;
+#endif
+
+    //  Virtual functions that are overriden in tests
+    virtual int do_getaddrinfo (const char *node_,
+                                const char *service_,
+                                const struct addrinfo *hints_,
+                                struct addrinfo **res_);
+
+    virtual void do_freeaddrinfo (struct addrinfo *res_);
+
+    virtual unsigned int do_if_nametoindex (const char *ifname_);
+};
+}
+
+#endif

src/tcp_address.hpp

@@ -35,6 +35,8 @@
 #include <netinet/in.h>
 #endif
 
+#include "ip_resolver.hpp"
+
 namespace zmq
 {
 class tcp_address_t
@@ -48,8 +50,7 @@ class tcp_address_t
     //  structure. If 'local' is true, names are resolved as local interface
     //  names. If it is false, names are resolved as remote hostnames.
     //  If 'ipv6' is true, the name may resolve to IPv6 address.
-    int
-    resolve (const char *name_, bool local_, bool ipv6_, bool is_src_ = false);
+    int resolve (const char *name_, bool local_, bool ipv6_);
 
     //  The opposite to resolve()
     virtual int to_string (std::string &addr_);
@@ -67,31 +68,8 @@ class tcp_address_t
     bool has_src_addr () const;
 
   protected:
-    int resolve_nic_name (const char *nic_, bool ipv6_, bool is_src_ = false);
-    int resolve_interface (const char *interface_,
-                           bool ipv6_,
-                           bool is_src_ = false);
-    int
-    resolve_hostname (const char *hostname_, bool ipv6_, bool is_src_ = false);
-
-#if defined ZMQ_HAVE_WINDOWS
-    int get_interface_name (unsigned long index, char **dest) const;
-    int wchar_to_utf8 (const WCHAR *src, char **dest) const;
-#endif
-
-    union
-    {
-        sockaddr generic;
-        sockaddr_in ipv4;
-        sockaddr_in6 ipv6;
-    } address;
-
-    union
-    {
-        sockaddr generic;
-        sockaddr_in ipv4;
-        sockaddr_in6 ipv6;
-    } source_address;
+    ip_addr_t address;
+    ip_addr_t source_address;
     bool _has_src_addr;
 };
 

tests/testutil.hpp

@@ -80,6 +80,8 @@
 #include <sys/socket.h>
 #include <netinet/in.h>
 #include <arpa/inet.h>
+#include <net/if.h>
+#include <netdb.h>
 #if defined(ZMQ_HAVE_AIX)
 #include <sys/types.h>
 #include <sys/socketvar.h>
@@ -412,6 +414,34 @@ int is_tipc_available (void)
 #endif // ZMQ_HAVE_TIPC
 }
 
+//  Wrapper around 'inet_pton' for systems that don't support it (e.g. Windows
+//  XP)
+int test_inet_pton (int af_, const char *src_, void *dst_)
+{
+#if defined(ZMQ_HAVE_WINDOWS) && (_WIN32_WINNT < 0x0600)
+    if (af_ == AF_INET) {
+        struct in_addr *ip4addr = (struct in_addr *) dst_;
+
+        ip4addr->s_addr = inet_addr (src_);
+
+        //  INADDR_NONE is -1 which is also a valid representation for IP
+        //  255.255.255.255
+        if (ip4addr->s_addr == INADDR_NONE
+            && strcmp (src_, "255.255.255.255") != 0) {
+            return 0;
+        }
+
+        //  Success
+        return 1;
+    } else {
+        //  Not supported.
+        return 0;
+    }
+#else
+    return inet_pton (af_, src_, dst_);
+#endif
+}
+
 #if defined(ZMQ_HAVE_WINDOWS)
 
 int close (int fd)

unittests/CMakeLists.txt

@@ -5,6 +5,7 @@ set(unittests
   unittest_ypipe
   unittest_poller
   unittest_mtrie
+  unittest_ip_resolver
 )
 
 #IF (ENABLE_DRAFTS)