LinuxQuestions.org - Need critique of C++ Socket class

- Programming (https://www.linuxquestions.org/questions/programming-9/)

- - Need critique of C++ Socket class (https://www.linuxquestions.org/questions/programming-9/need-critique-of-c-socket-class-706177/)

Need critique of C++ Socket class

Over the course of the years, I have pieced together a C++ OO socket library that I use (successfully) for TCP and UDP communications. When I bind() to a socket, and a hostname or IP address is given, I use gethostbyname().

Through a little research this morning, I discovered that the gethostbyname() function is considered "obsolete", and instead getaddrinfo() should be used. Is this correct?

The example code I have seen that describes the usage of getaddrinfo() does not work well with the design of the socket library I developed, namely because an attempt is made to create a socket using information returned by getaddrinfo(). In my code, the socket has already been created before I ever attempt to bind() to a socket.

Below is a snippet of my code, which employs gethostbyname(). Can someone please provide some ideas as to how I should approach incorporating the getaddrinfo() into my code, or even if I should?

Should I defer opening/creating the socket until the server application calls bind()? Or in the case of a client application, until that application calls connect()?

I am not too keen on defering the opening of the socket from the Socket() constructor, and thus I wonder if the implementation I have is acceptable?

Anyhow, any comments are appreciated -- but please keep them in layman's terms!

Code:

////////////////////////////////////////////////////////////////////////////////

//

// Method:  Socket (constructor)

//

////////////////////////////////////////////////////////////////////////////////

Socket::Socket(int domain, int type, int protocol) :

              m_Domain(domain),

              m_Type(type),

              m_Protocol(protocol)

{

  if ((m_Socket = socket(m_Domain, m_Type, m_Protocol)) < 0)

  {

    throw std::runtime_error("Could not create socket!");

  }

}



...



////////////////////////////////////////////////////////////////////////////////

//

// Method:  Bind

//

////////////////////////////////////////////////////////////////////////////////

void

Socket::Bind(const std::string& address, unsigned short port)

{

  sockaddr_in addr;



  FillAddress(address, port, addr);



  if (bind(m_Socket, (sockaddr*) &addr, sizeof(sockaddr_in)) < 0)

  {

    throw std::runtime_error("Could not bind to socket!");

  }

}





////////////////////////////////////////////////////////////////////////////////

//

// Method:  FillAddress

//

////////////////////////////////////////////////////////////////////////////////

void

Socket::FillAddress(const std::string& address, unsigned short port, sockaddr_in& addr)

{

  hostent* host = gethostbyname(address.c_str());



  if (host == 0)

  {

    throw std::runtime_error("Could not get hostname from given address!");

  }



  memset(&addr, 0, sizeof(addr));



  addr.sin_family      = m_Domain;

  addr.sin_addr.s_addr = *((unsigned long *) host->h_addr_list[0]);

  addr.sin_port        = htons(port);

}

I have a TCPServerSocket, a TCPSocket (for clients), and a UDPSocket that subclass from Socket. Here's how I would typically use the TCPServerSocket in an application:

Code:

...



  const unsigned short port = 9100;

  TCPServerSocket      server;



  server.Bind("192.168.6.128", port);

  server.Listen();

  server.EnableReuseAddress();



...

Maybe this? Sorry, I didn't have a chance to compile or test it, but it's based on something I use.

Code:

Socket::FillAddress(const std::string& address, unsigned short port, sockaddr_in& addr)

{

  struct addrinfo *host_info = NULL;



  if ( getaddrinfo(address.c_str(), NULL, NULL, &host_info) != 0 ||

      !host_info || !host_info->ai_addr || host_info->ai_family != AF_INET )

  {

    if (host_info) freeaddrinfo(host_info);

    throw std::runtime_error("Could not get hostname from given address!");

  }



  memcpy(&addr, host_info->ai_addr, sizeof(struct sockaddr_in));

  addr.sin_port = htons(port);



  freeaddrinfo(host_info);

}

I'd avoid sizeof addr because of its ambiguity, although I'm not up on how the standard deals with sizes of references. Also, sizeof has parentheses with type names but not with variable names.

Quote:

Originally Posted by dwhitney67 (Post 3451007)

Should I defer opening/creating the socket until the server application calls bind()? Or in the case of a client application, until that application calls connect()?

I'm not sure what you mean by this. Addressing doesn't take place during socket creation; only during binding and connecting.

Do you think throwing an exception is appropriate? I know that's how things are dealt with in Java, but with something that incorporates low-level C it might not be wise. Also, it creates additional overhead and the inconvenience of try/catch when you could just be returning false.

In all, though, I think it's a good idea to have a socket class. I generally just use the same 20 lines of socket code in each new place. I use C whenever possible for that sort of thing, though, so a class isn't really an option for me.

I've never actually worked with an IPv6 network before; however, using getaddrinfo would make adding it as a consideration very simple. You might be able to get away with not using a template if you go about it C-style and use sockaddr* and sizes as private-function arguments.
Kevin Barry

ta0kira

Thank you very much. I employed the code you provided in your previous post within the my FillAddress() method, and it worked as expected.