How to set up a std container with a RAII class?

Question

To realize the raii idiom for the type SOCKET, I have created a wrapper. The wrapper calls connect in the constructor and closesocket in its destructor. A std::map holds all used sockets. Unfortunatelly inserting a new socket into the container calls the destructor of a temporary and, in fact, closes the just opened socket. Is there a common way to overcome this?

Here is the code:

#include <iostream>
#include <stdexcept>
#include <map>
#include <winsock2.h>

struct Socket {
    SOCKET mSock;
    Socket() : mSock(INVALID_SOCKET) {}
    Socket(std::string ip, int port);
    ~Socket();
};

Socket::Socket(std::string ip, int port) {
    mSock = socket(AF_INET, SOCK_STREAM, 0);
    if (mSock == INVALID_SOCKET)
        throw std::runtime_error("socket()");
    SOCKADDR_IN addr = {0};
    addr.sin_family = AF_INET;
    addr.sin_port = htons(port);
    addr.sin_addr.s_addr = inet_addr(ip.c_str());
    if (connect(mSock, reinterpret_cast<SOCKADDR*>(&addr), sizeof(addr))
        == SOCKET_ERROR)
        throw std::runtime_error("connect()");
    std::cout << mSock << " connected" << std::endl;
}

Socket::~Socket() {
    if (mSock != INVALID_SOCKET) {
        closesocket(mSock);
        std::cout << mSock << " closed" << std::endl;
    }
}

int main() {
    WSADATA wsa;
    WSAStartup(MAKEWORD(2, 0), &wsa);
    std::map<int, Socket> outbound;
    // calls constructur but also destructor
    outbound[0] = Socket("192.168.128.125", 4023);
    WSACleanup();
    return 0;
}

And the Output is:

1952 connected
1952 closed
1952 closed

Use move semantics and delete `Socket`'s copy contructor/assignment. When you think about it, it makes no sense to copy a socket. — syam, Jul 25 '13 at 17:05
First of all, you must make it **noncopyable**, though it should be **moveable**. — Nawaz, Jul 25 '13 at 17:07
@syam: I can't use move semantics because: I've tried to convert this VC6 project into a Visual Studio 2010 project but this was not possible because of dependencies to linked VC6 libraries. — Christian Ammer, Jul 25 '13 at 17:08
I believe you can also do something like `outbound[0].connect("192.168.128.125", ...)`, if a `connect` method were implemented. And as syam said, make the copy constructor/assignment operator private to prevent the error you noticed. — Rollie, Jul 25 '13 at 17:13
@ChristianAmmer Ah ok. Then don't store the object itself, but a pointer/reference to it (as Yury said, smart pointers are a good choice). Still, *do* make your class non copyable so that you can detect such issues at compilation time. — syam, Jul 25 '13 at 17:13
@Rollie: Your suggestion makes sense to me. If you turn your comment into a answer, I would vote for it. — Christian Ammer, Jul 25 '13 at 17:18

score 4 · Accepted Answer · answered Jul 25 '13 at 17:11

Resource guards may be stored in STL containers if you wrap them in a smart pointer. Which smart pointer to use exactly is up to your environment, I'd recommend to go with boost::shared_ptr as a starting point if you're not sure.

Following that way you will conform with both resource guard semantics (only one instance that manages resource's lifetime) and STL container (keep equivalent copy of passed item).

score 1 · Answer 2 · answered Jul 25 '13 at 18:52

As others have mentioned, you should be preventing the copying of your Socket object, which can be done by adding:

private:
  Socket(const Socket &){}
  Socket & operator=(const Socket &){return *this;}

Then to actually connect, you could do so after default construction with the implementation of a connect method, which could be called explicitly, or by the remaining constructor:

bool Socket::connect(std::string ip, int port) {
    mSock = socket(AF_INET, SOCK_STREAM, 0);
    if (mSock == INVALID_SOCKET)
        return false;
    SOCKADDR_IN addr = {0};
    addr.sin_family = AF_INET;
    addr.sin_port = htons(port);
    addr.sin_addr.s_addr = inet_addr(ip.c_str());
    if (connect(mSock, reinterpret_cast<SOCKADDR*>(&addr), sizeof(addr))
        == SOCKET_ERROR)
        return false;
    std::cout << mSock << " connected" << std::endl;
    return true;
}

Socket::Socket(std::string ip, int port) {
    if (!connect(ip, port))
        throw std::runtime_error("socket()");
}

I was in good hope, that this works but the line `outbound[0].init("192.168.128.125", 4023)` results in the error C2248 (cannot access 'access' member...). It seemed to me that non copyable items can't be inserted into a map, and that's really the reason as I later found a explanation in [this answer](http://stackoverflow.com/a/1440328/237483). — Christian Ammer, Jul 25 '13 at 21:26

How to set up a std container with a RAII class?

2 Answers2