3

Assuming the following code:

class ChannelHandle;
class SessionHandle;

typedef ChannelHandle& ChannelHandleRef;
typedef SessionHandle& SessionHandleRef;

class RemoteChannelHandle
{
public:
    RemoteChannelHandle(
        ChannelHandleRef pChannelHandle, SessionHandleRef pSessionHandle);
    RemoteChannelHandle(RemoteChannelHandle&&);
    ~RemoteChannelHandle();

    void CloseChannel();

#ifndef _MSC_VER
    RemoteChannelHandle& operator=(RemoteChannelHandle&&) = delete;
    RemoteChannelHandle(RemoteChannelHandle const&) = delete;
    RemoteChannelHandle& operator=(RemoteChannelHandle const&) = delete;
#endif
private:
    LIBSSH2_CHANNEL* channel;
    ChannelHandleRef channelHandle;
    SessionHandleRef sessionHandle;
};

RemoteChannelHandle::RemoteChannelHandle(
    ChannelHandleRef pChannelHandle, SessionHandleRef pSessionHandle)
    : channel(nullptr), channelHandle(pChannelHandle), sessionHandle(pSessionHandle)
{
    // OPEN SSH CHANNEL
}

RemoteChannelHandle::~RemoteChannelHandle()
{
    try
    {
        CloseChannel();
    }
    catch (...)
    { }
}

void RemoteChannelHandle::CloseChannel()
{
    if (channel == nullptr)
    {
        return;
    }

    // CLOSE SSH CHANNEL. THROW IF SOMETHING GOES WRONG

    channel = nullptr;
}
  • RemoteChannelHandle opens an SSH channel, but the cleanup requires two steps (close + free). The first step is performed in ~RemoteChannelHandle(), but the second will be performed in ChannelHandle's dtor; hence the data member channelHandle, since we'll need to inject channel into it. This reference could be eliminated by performing both steps in ~RemoteChannelHandle().
  • sessionHandle holds the LIBSSH2_SESSION* necessary to open the SSH channel. Since I don't want to pass LIBSSH2_SESSION* around, this reference can't be eliminated.

The problem happens when I define a move ctor for RemoteChannelHandle. I need to clear the members of the "moved from" instance. However, there's no way to clear the reference. So, what to do here?

  1. Use (const) std::shared_ptr instead of references? I could even use naked pointers, as there's no ownership involved. I realize there's some debate regarding the use of references as data members, but apart from the move ctor, I foresee no other scenario where I would need to reseat the handles (which is why I used references in the first place).
  2. Leave the references as they are, and create a "state" data member specifically to check if the object is in a valid state (I can use channel != nullptr for this purpose)?
  3. Any other idea?

I've searched for alternatives to this, and found no clear answer. Which could mean there's actually no clear answer, of course.

Thanks for your time.

Edit (in reply to Mankarse): ChannelHandle exists solely to perform step 2 of cleanup. Here's a simplified definition:

class ChannelHandle
{
public:
    ChannelHandle();
    ChannelHandle(ChannelHandle&&);
    ~ChannelHandle();

#ifndef _MSC_VER
    ChannelHandle& operator=(ChannelHandle&&) = delete;
    ChannelHandle(ChannelHandle const&) = delete;
    ChannelHandle& operator=(ChannelHandle const&) = delete;
#endif

    LIBSSH2_CHANNEL* GetChannel() { return channel; }
    void SetChannel(LIBSSH2_CHANNEL* pChannel) { channel = pChannel; }

    void FreeChannel();
private:
    LIBSSH2_CHANNEL* channel;
};

SessionHandle is an RAII encapsulation for a LIBSSH2_SESSION*. It calls libssh2_session_init() on ctor and libssh2_session_free() on dtor. Other similar classes take care of other steps of the SSH session's init/cleanup, but this is where we get the LIBSSH2_SESSION* from libssh2, and SessionHandle owns it. Once again, a simplified definition:

class SessionHandle
{
public:
    SessionHandle();
    SessionHandle(SessionHandle&&);
    ~SessionHandle();

    void CloseSession();
    LIBSSH2_SESSION* GetSession() { return session; }

#ifndef _MSC_VER
    SessionHandle& operator=(SessionHandle&&) = delete;
    SessionHandle(SessionHandle const&) = delete;
    SessionHandle& operator=(SessionHandle const&) = delete;
#endif
private:    
    LIBSSH2_SESSION *session;
};
PCaetano
  • 581
  • 5
  • 11
  • How are `ChannelHandle` and `SessionHandle` defined, and what do they do? – Mankarse Jan 24 '13 at 11:38
  • Since you have a lvalue reference member, the lvalue reference and the classes containing it is not a temporary. The move ctor may never get invoked anyway no matter how you define it. The copy ctor may get picked all the time anyway. – emsr Jan 24 '13 at 12:34
  • see this answer http://stackoverflow.com/a/8706254/819272 – TemplateRex Jan 24 '13 at 14:08
  • @emsr You could be right, I'm still in the early stages of using these classes, so my tested scenarios are still limited. From my tests with gcc (tests with VC++ to follow), return by value invokes nothing, so it's probably RVO at work. The scenario I'm testing now, and for which I wanted the move ctor, is moving objects of these classes from/to containers. – PCaetano Jan 24 '13 at 14:23
  • @rhalbersma Well, two "votes" for pointers instead of references in this scenario. – PCaetano Jan 24 '13 at 14:35
  • references if you can, pointers if you must – TemplateRex Jan 24 '13 at 14:46

1 Answers1

3

You've answered the question yourself:

I could even use naked pointers, as there's no ownership involved.

Mike Seymour
  • 249,747
  • 28
  • 448
  • 644
  • True, but am I wrong in assuming that, if it wasn't for the move ctor, implementation with references would be the Right Way(TM)? I've been away from C++ for a few years, but back when I learned it "Always prefer references" was mantra-like. – PCaetano Jan 24 '13 at 14:33
  • 1
    @PCaetano: References are better than pointers if you don't need to nullify them or change the target, because they prevent you from accidentally doing those things. But if you do need to do those things (as you do here), then you can't use them. – Mike Seymour Jan 24 '13 at 14:41
  • 2
    Using references eliminates ambiguity, whereas pointers require something more - e.g., a comment, a typedef, a wrapper. I've just read proposal N3514, and it could be a step in the right direction, by creating a standard way of expressing this relationship. – PCaetano Jan 24 '13 at 17:45