HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

C++ getaddrinfo()/addrinfo wrapper (rewrite)

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
wrapperrewritegetaddrinfoaddrinfo

Problem

As suggested by @Dannnno on my original post, I am posting my partially rewritten getaddrinfo() (and now addrinfo) wrapper for merciless review. I've made quite a few changes to the code as suggested originally, and included a minimal example of main() to show how it comes together.

Some things I'd like to note:

-
I decided not to expose a next method in my AddrInfo wrapper for now, since it's only used for getaddrinfo() results. I thought about making the iterator built into the AddrInfo class instead of GetAddrInfo, but decided against it.

-
I'm not sure if I did my assignment operators correctly in getaddrinfo(), or if there's even a better approach to writing them (I am not entirely happy with the duplication of code).

-
I've targeted this at Unix for now, but I'm open to fixes for portability to Windows and the like (but not necessarily asking for them).

EDIT: Incorporated some changes suggested to the original, but it doesn't change the underlying code substantially.

Here's the code:

```
#include
#include
#include
#include
#include

#include
#include
#include
#include
#include
#include

class AddrInfo
{
private:
struct addrinfo *next_;
public:
int flags = 0;
int addr_family = 0;
int socket_type = 0;
int protocol = 0;
socklen_t address_len = 0;
sockaddr *address = nullptr;
char *canonical_name = nullptr;

const addrinfo to_addrinfo()
{
addrinfo addr = {};

addr.ai_flags = flags;
addr.ai_family = addr_family;
addr.ai_socktype = socket_type;
addr.ai_protocol = protocol;
addr.ai_addrlen = address_len;
addr.ai_addr = address;
addr.ai_canonname = canonical_name;
addr.ai_next = next_;

return addr;
}

std::pair to_pair()
{
char str_addr[INET6_ADDRSTRLEN] = {};
void *sock_ptr;

if(addr_family == AF_INET)
sock_ptr = &(((sockaddr_in *)address)->sin_addr);
else

Solution

It seems to me that GetAddrInfo is sort of a class without a cause, so to speak. Essentially all it really does is handle the mechanics of its own existence--initial creation, copy construction, move construction, assignment, etc. Essentially the only things you can really do with a GetAddrInfo are call its begin and end (or cbegin/cend) to get iterators into an underlying sequence.

I'd rather get rid of the GetAddrInfo object, and just create iterators directly.

In somewhat simplified form, the idea could look something like this:

#include 
#include 
#include 
#include 
#include 
#include 

#ifdef _WIN32
#include 
#include 

class IP_user {
    WSADATA d;
public:
    IP_user() {
        WSAStartup(MAKEWORD(2, 2), &d);
    }

    ~IP_user() {
        WSACleanup();
    }
} init; 

#else
#include 
#include 
#include 
#include 
#include 
#endif

namespace addr {
    using T = std::pair;

    class iterator : public std::iterator {
        struct addrinfo *start;
        struct addrinfo *address;
public:

        iterator(std::string const &node_name, std::string const &service_name = "", addrinfo *hints = nullptr) {
            getaddrinfo(node_name.c_str(), service_name.c_str(), hints, &address);
            start = address;
        }

        iterator() : start(nullptr), address(nullptr) { }

        iterator(iterator &&other) 
            : start(other.start), 
            address(other.address) 
        {
            other.start = nullptr;
        }

        iterator operator=(iterator &&other) = delete;
        iterator(iterator const &other) = delete;
        iterator operator=(iterator const &other) = delete;

        iterator &operator++() {
            address = address->ai_next;
            return *this;
        }

        bool operator == (iterator const &other) { 
            return address == other.address;
        }

        bool operator !=(iterator const &other) { 
            return address != other.address;
        }

        T operator*() const { 
            static const std::map names{
                { AF_INET, "IPv4" },
                { AF_INET6, "IPv6" }
            };

            char str_addr[INET6_ADDRSTRLEN] = {};       

            std::string res;
            if (address->ai_family == AF_INET)
                res = inet_ntoa(((sockaddr_in *)address->ai_addr)->sin_addr);
            else
                res = inet_ntop(address->ai_family, address->ai_addr, str_addr, sizeof(str_addr));

            return std::make_pair(names.find(address->ai_family)->second, std::string(res));
        }

        ~iterator() { 
            if (start != nullptr)
                freeaddrinfo(start); 
        }
    };
}

int main(int argc, char **argv) { 
    for (addr::iterator b(argv[1]), e; b != e; ++b)
        std::cout << (*b).first << ": " << (*b).second << "\n";
}

Code Snippets

#include <iterator>
#include <iostream>
#include <string>
#include <utility>
#include <map>
#include <algorithm>

#ifdef _WIN32
#include <winsock2.h>
#include <ws2tcpip.h>

class IP_user {
    WSADATA d;
public:
    IP_user() {
        WSAStartup(MAKEWORD(2, 2), &d);
    }

    ~IP_user() {
        WSACleanup();
    }
} init; 

#else
#include <sys/types.h>
#include <sys/socket.h>
#include <netdb.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#endif

namespace addr {
    using T = std::pair<std::string, std::string>;

    class iterator : public std::iterator<std::input_iterator_tag, T> {
        struct addrinfo *start;
        struct addrinfo *address;
public:

        iterator(std::string const &node_name, std::string const &service_name = "", addrinfo *hints = nullptr) {
            getaddrinfo(node_name.c_str(), service_name.c_str(), hints, &address);
            start = address;
        }

        iterator() : start(nullptr), address(nullptr) { }

        iterator(iterator &&other) 
            : start(other.start), 
            address(other.address) 
        {
            other.start = nullptr;
        }

        iterator operator=(iterator &&other) = delete;
        iterator(iterator const &other) = delete;
        iterator operator=(iterator const &other) = delete;

        iterator &operator++() {
            address = address->ai_next;
            return *this;
        }

        bool operator == (iterator const &other) { 
            return address == other.address;
        }

        bool operator !=(iterator const &other) { 
            return address != other.address;
        }

        T operator*() const { 
            static const std::map<int, std::string> names{
                { AF_INET, "IPv4" },
                { AF_INET6, "IPv6" }
            };

            char str_addr[INET6_ADDRSTRLEN] = {};       

            std::string res;
            if (address->ai_family == AF_INET)
                res = inet_ntoa(((sockaddr_in *)address->ai_addr)->sin_addr);
            else
                res = inet_ntop(address->ai_family, address->ai_addr, str_addr, sizeof(str_addr));

            return std::make_pair(names.find(address->ai_family)->second, std::string(res));
        }

        ~iterator() { 
            if (start != nullptr)
                freeaddrinfo(start); 
        }
    };
}

int main(int argc, char **argv) { 
    for (addr::iterator b(argv[1]), e; b != e; ++b)
        std::cout << (*b).first << ": " << (*b).second << "\n";
}

Context

StackExchange Code Review Q#135751, answer score: 2

Revisions (0)

No revisions yet.