Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Migrating code from C++03 to C++11: should I be cautious about the implicit default move constructor?

Tags:

c++

c++11

c++03

I have a codebase that I'd like to switch from C++03 to C++11.

As I understand, some classes will benefit from the change by having an implicit default move constructor (and the move assignment operator that comes along). While I'm totally ok with that (I even think it is a nice thing) I'm a bit afraid of the effects that such implicit constructors might have on some non-copyable classes I have.

One example I have is a class that wraps an iconv_t handle from libiconv to leverage RAII.

More explicitely, the class is as follow:

class iconv_wrapper
{
   public:
     iconv_wrapper() : m_iconv(iconv_open()) {}
     ~iconv_wrapper() { iconv_close(m_iconv); }
   private:
     // Not implemented: non copyable. Should probably be = delete; in C++11.
     iconv_wrapper(const iconv_wrapper&);
     iconv_wrapper& operator=(const iconv_wrapper&);

     iconv_t m_iconv;
};

My concern is: if an instance of this class happened to be moved, this would result in a double call to iconv_close(). As iconv_t is a "dumb" integral type, I don't expect the default implementation of iconv_wrapper(iconv_wrapper&&) to nullify the m_iconv member of the R-value. Even if it did, the destructor is not implemented to handle this properly.

So my questions are:

  • Are my concerns legitimate ? Am I right that the default movable constructor/operator= will be incorrect in such a case ?
  • What changes can I make to port this code nicely to C++11 ? (I was suggested std::unique_ptr but I couldn't make this work nicely, as it expects a pointer, not some opaque type)
like image 310
ereOn Avatar asked Jan 14 '14 20:01

ereOn


1 Answers

It won't be moved. Because you have a user declared copy constructor, copy assignment operator and destructor, the move constructor and move assignment operator will not be generated. Actually, any one of those three declared would suppress automatic generation of the move constructor and move assignment operator.

If you want to make it more C++11 friendly, you could add a move constructor and move assignment operator, as in (warning: never compiled or tested):

class iconv_wrapper
{
   public:
     iconv_wrapper() : m_iconv(iconv_open()) {}
     ~iconv_wrapper() { if ((iconv_t)-1 != m_iconv) iconv_close(m_iconv); }
     iconv_wrapper(iconv_wrapper&& that) noexcept { std::swap(m_iconv, that.m_iconv); }
     iconv_wrapper& operator=(iconv_wrapper&& that) noexcept { std::swap(m_iconv, that.m_iconv); return *this; }

   private:
     iconv_t m_iconv = (icontv_t)-1;
};

A reason you might want to do this is so you can store these objects (or other types which contain these objects) in a vector.

like image 77
Nevin Avatar answered Sep 17 '22 21:09

Nevin