Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Use of placement-new operator and copy constructor instead of assignment operator

I found a problem while using 3rd party code which cannot be altered. I need to make a copy of object member. I can't do this strictly because one of inner members has private assignment operator. The only solution I found is tricky so I want to ask you if you see any red lights that can affect my program.

Here's the simplified code I'm dealing with (remember that I cannot change it!):

#include <iostream>
#include <algorithm>

class MBool
{
public:
    MBool() {};
    MBool(const MBool& arg) {}
private:    
    MBool& operator=(const MBool& arg);
};

class InnerContent {
private:
    int* pBuffer;

public: 
    InnerContent() {
        pBuffer = new int[20];
        std::cout << "InnerContent()" << std::endl;
    }

    InnerContent(const InnerContent& otherInnerContent) {
        pBuffer = new int[20];
        std::copy(otherInnerContent.pBuffer, otherInnerContent.pBuffer + 20, pBuffer);
        std::cout << "InnerContent(const InnerContent&)" << std::endl;
    }

    ~InnerContent() {
        std::cout << "~InnerContent()" << std::endl;
        delete [] pBuffer;
        pBuffer = nullptr;
    }

    virtual void someVirtualFunction() {}
};

class Content {
public:
    InnerContent innerContent;
    int someNumber;
    MBool boolVar;

    Content() {
        std::cout << "Content()" << std::endl;
    }
    ~Content() {
        std::cout << "~Content()" << std::endl;
    }
    Content(const Content& otherContent) :
        innerContent(otherContent.innerContent),
        someNumber(otherContent.someNumber),
        boolVar(otherContent.boolVar)
    {
        std::cout << "Content(const Content&)" << std::endl;
    }

    virtual void someVirtualFunction() {}
};

class A {
public: 
    Content content;

    A() { std::cout << "A()" << std::endl; }
    ~A() { std::cout << "~A()" << std::endl; }
};

class B {
public: 
    Content content;

    B() { std::cout << "B()" << std::endl; }
    ~B() { std::cout << "~B()" << std::endl; }
};

And here's what I'm about to do with it (only this code can be modified and extended):

void copyContent(Content& contentFrom, Content& contentTo) {
    contentTo.~Content();
    new (&contentTo) Content(contentFrom);
};

int main() {
    A a;
    B b;

    // I wish to do this:
    //b.content = a.content;
    // but Content class has no operator= function implemented
    // also I can't use generated assignment operator function because of MBool::operator= is private

    // The only work-around I found is this:

    std::cout << "--- Before copying" << std::endl;
    copyContent(a.content, b.content);
    std::cout << "--- After copying" << std::endl;
}

My solution is to call Content destructor manually to free any dynamically allocated memory in Content and its inner classes. Memory on the stack remains untouched so I can reuse it with placement-new operator that calls copy constructor that is present and does exactly what I need. When main function scope ends 'a' object is cleaned up properly.

Code output:

InnerContent()
Content()
A()
InnerContent()
Content()
B()
--- Before copying
~Content()
~InnerContent()
InnerContent(const InnerContent&)
Content(const Content&)
--- After copying
~B()
~Content()
~InnerContent()
~A()
~Content()
~InnerContent()

I don't want to make my own function that copies all the fields because this class can be updated in new version and there may be additional field that I will not copy and most probably no one will remember to fix it.

Question: Do you think this may cause any memory leaks or memory corruption? Do you see any problems that I didn't mention?

like image 576
Szymon Kordyaczny Avatar asked Oct 29 '22 20:10

Szymon Kordyaczny


1 Answers

Basically the Idea should work. To protect yourself from forgetting to call the destructor, I think, you should wrap the whole think in a kind of smart pointer like class template. In this example it actually does not wrap a pointer, but the content object itself.

template <typename ContentType>
class content_wrapper {
    private:
        ContentType content_;
    public:
        content_wrapper() : content_ {} {};
        content_wrapper(const content_wrapper& other) :
            content_{other.content_} {};

        content_wrapper& operator = (const content_wrapper& other) {
            content_.~ContentType();
            new (&content_) ContentType(other);
            return *this;
        }

        ContentWrapper& operator * () {
            return content_;
        }
        ContentWrapper* operator -> () {
            return &content_;
        }
};

now you can use it like that:

class A {
    public: 
        content_wrapper<Content> content;

        A() { std::cout << "A()" << std::endl; }
        ~A() { std::cout << "~A()" << std::endl; }
};

class B {
    public: 
        content_wrapper<Content> content;

        B() { std::cout << "B()" << std::endl; }
        ~B() { std::cout << "~B()" << std::endl; }
};

int main() {
    A a;
    B b;

    b.content = a.content; // the wrapper will take care.

    b.content->someVirtualFunction();
}

Easy to read and you can never forget the destructor call, whenever you want to assign a content object.

like image 53
cdonat Avatar answered Nov 15 '22 05:11

cdonat