5
votes

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?

1
Instead just use a smart pointer to Content. Easy to replace its pointee with a new one.Cheers and hth. - Alf
I don't see any problems per se, just make sure the copy constructor stays up to date, or you will have problems.......I have actually used similar strategies to this beforeDarthRubik
What happens when you do copyContent(x,x)?Barry
At first glance it appears the library is designed for that class to not be assignable and you can't predict what may or may not happen. Are you sure that your code/design requires assigning this type, or is there perhaps an alternate mechanism you could use?Mark B
@Cheersandhth.-Alf I wish I could do that. I cannot modify any of classes from the example: A, B, Content, InnerContent (all of those come from library). I use smart pointers to instances of A and B.Szymon Kordyaczny

1 Answers

2
votes

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.