4
\$\begingroup\$

I have a C++03 library with lots of classes. I'm interested in trying to take advantage of Rvalues and move semantics under C++11 for two or three critical classes. Once of the critical classes is a SecBlock, which is effectively a secure array class. It has a secure allocator and deallocator, and its used nearly everywhere.

SecBlock looks like so, where T might be a byte or an int:

template <class T, class A = AllocatorWithCleanup<T> >
class SecBlock
{
   ...
private:
    A m_alloc;
    size_type m_size;
    T *m_ptr;
}

If I am reading Thomas Becker's "C++ Rvalue References" treatise correctly, I can use a simple swap because SecBlock does not have a base class (no need for Base(std::move(rhs)):

template <class T, class A = AllocatorWithCleanup<T> >
class SecBlock
{
   ...

#if CXX11_AVAILABLE
    SecBlock(SecBlock&& t)
    {
        std::swap(*this, t);
    }
    SecBlock& operator=(SecBlock&& t)
    {
        if(this != &t)
            std::swap(*this, t);
        return *this;
    }
#endif

private:
    A m_alloc;
    size_type m_size;
    T *m_ptr;
}

We guard on CXX11_AVAILABLE because the library is a C++03 library at heart.

It's probably obvious I have some open questions. For instance, do I have to guard for self assignment in operator=? (I suspect not, but I think its best to show the code and take the comments).

I'm also interested in knowing if a derived class must implement the semantics also, even if they add no members.

Is the code above correct, or are there gaps that need to be addressed? If there are gaps, what are they?

\$\endgroup\$

closed as off-topic by Martin York, Hosch250, Barry, Quill, jacwah Aug 1 '15 at 10:06

This question appears to be off-topic. The users who voted to close gave these specific reasons:

  • "Questions containing broken code or asking for advice about code not yet written are off-topic, as the code is not ready for review. After the question has been edited to contain working code, we will consider reopening it." – Martin York, Barry, jacwah
  • "Questions must involve real code that you own or maintain. Pseudocode, hypothetical code, or stub code should be replaced by a concrete example. Questions seeking an explanation of someone else's code are also off-topic." – Hosch250, Quill
If this question can be reworded to fit the rules in the help center, please edit the question.

  • \$\begingroup\$ Note crypto++ is synonymized with cryptography. \$\endgroup\$ – Mast Jul 31 '15 at 21:16
  • \$\begingroup\$ @Mast: they are two different things. One is a field of study, the other is a software library. Intuitively, they cannot be used interchangeably in a sentence, so they are clearly not synonyms. \$\endgroup\$ – user53032 Jul 31 '15 at 21:22
  • 3
    \$\begingroup\$ This discussion has been held before: Current synonymization of crypto++. We will not have this discussion in comments. \$\endgroup\$ – Mast Jul 31 '15 at 21:26
  • \$\begingroup\$ @Mast - I'm not interested in a discussion about it. In the previous question, I simply asked what I should do because I was receiving inappropriate/unwanted emails due to the intersection with the notification system. The discussion you folks had about it was irrelevant to me because you discussed something I did not ask. In this question I simply pointed out the errant behavior to let folks know it was not me mis-classifying the question. In both cases, I'm not interested in a discussion. \$\endgroup\$ – user53032 Jul 31 '15 at 21:34
  • \$\begingroup\$ Somewhat related, definitely worth the read, nevertheless: stackoverflow.com/a/3279550/1198654 \$\endgroup\$ – glampert Aug 1 '15 at 1:12
3
\$\begingroup\$

This code does not function.

Let's take a look at your move constructor:

SecBlock(SecBlock&& t)
{
    std::swap(*this, t);
}

This caused a stack overflow on my system. Why? Imagine if the swap function was implemented like this:

template <typename T>
void swap (T &t1, T &t2)
{
    T temp = std::move (t1) ;
    t1 = std::move (t2) ;
    t2 = std::move (temp) ;
}

In fact, this is how it is implemented by Visual Studio 2012. The first line T temp = std::move (t1) ;, calls your move constructor. This causes you to recursively call your move constructor until your program inevitably crashes.

Another problem is that before the swap, m_size and m_ptr for *this will be uninitialized. So even if you fixed your move constructor, you will be transferring uninitialized data to t. Assuming you're destructor does some clean-up, this could lead to undefined behavior.

All in all, both member-functions are a mess. Check out the rule of 5 for more help.

\$\endgroup\$
  • \$\begingroup\$ Now I'm in a bad spot. The question may violate the site's policies on working, bug free code. What should I do with it? Delete it, or leave it and accept your answer? (Sorry to ask. I don't make it to these parts often). \$\endgroup\$ – user53032 Jul 31 '15 at 22:27
  • \$\begingroup\$ @jww Give it time I guess. If it violates this site's rules, then I imagine the question will eventually be closed. If it doesn't violate the rules, then wait for more answers and select the best one. \$\endgroup\$ – ncalmbeblpaicr0011 Jul 31 '15 at 22:29
  • \$\begingroup\$ "This code does not function...." - oh, my. It tested OK on OS X. I did not start a testing cycle on Lnux, BSD or Windows. I was waiting for feedback before the testing cycle. Thank you very much. I just learned a tremendous amount about these things... \$\endgroup\$ – user53032 Jul 31 '15 at 22:30
  • \$\begingroup\$ I see what happened here.... Apple/Clang claimed 201103L, but failed a feature test. When it failed a feature test, the code path was not activated. When I cut-int std::move without the guards to test your findings, it could not even compile. OS X is the pits. I'm jumping over to Fedora 22 and the GCC 5.1 compiler. \$\endgroup\$ – user53032 Jul 31 '15 at 23:51
  • 1
    \$\begingroup\$ @jww, if an answer finds a bug in your code, it doesn't automatically makes the question of-topic (in fact it happens quite often that bugs are found by reviews!) If to the best of the asker's knowledge the code is working, then it is on topic. This is the case with your question. You thought it was correct, but a review found a bug. Great thing you asked for code review! \$\endgroup\$ – glampert Aug 1 '15 at 1:10