0

I have a simple filesystem containing either a file or a directory. I have created an abstract class as a parent of both of these classes + two functions Size() and Change() as can be seen here:

class AbstractParent {
public:
    explicit AbstractParent (const string & name = "", const unsigned int size = 0) : cString(name), cSize(size) {};
    virtual ~AbstractParent() = default;
    virtual unsigned int Size() const = 0;
    unique_ptr<AbstractParent> Clone() const {
        unique_ptr<AbstractParent> other(this->deepClone());
        return other;
    }

protected:
    string cString;
    unsigned int cSize;
    //virtual copy constructor
    virtual AbstractParent * deepClone() const = 0;
};

class CFile : public AbstractParent 
{
public:
    // constructor
    explicit CFile (const string & hash, const unsigned int size) : AbstractParent(hash,size) {};
    // Size
    unsigned int Size() const override{
        return cSize;
    }
    // Change
    CFile & Change (const string & newHash, const unsigned int newSize) {
        cString = newHash;
        cSize = newSize;
        return *this;
    }
    //virtual copy constructor
    CFile * deepClone () const override{
        return new CFile(*this);
    }
};

class CDirectory : public AbstractParent
{
public:
    // constructor
    explicit CDirectory () {
        systemMap = new map<string, unique_ptr<IFileSystem>>;   //valgrind reports a leak here
    };
    // Size
    unsigned int Size () const override {
        return cSize;
    }
    // Change
    CDirectory & Change (const string & FSName,const IFileSystem & file) {
        systemMap->insert(make_pair(FSName, file.Clone()));
        return *this;
    }
    //virtual copy
    CDirectory * deepClone () const override {
        return new CDirectory(*this);
    }
private:
    map<string, unique_ptr<IFileSystem>> * systemMap;
};

I have a problem with insering a directory into another directory. I would like to use a map for the directory, where the key is the name of the file/directory and its value is a pointer to the instance of it.

Here I have attempted to make it function with a virtual copy constructor and while it does compile, it does not work properly. Valgrind is reporting lost memory where I allocate new map.

code in main to test out my implementation:

int main ()
{
    CDirectory root;
    stringstream sout;

    root.Change("file.txt", CFile("jhwadkhawkdhajwdhawhdaw=", 1623))
            .Change("folder", CDirectory()
                    .Change("fileA.txt", CFile("", 0).Change("skjdajdakljdljadkjwaljdlaw=", 1713))
                    .Change("fileB.txt", CFile("kadwjkwajdwhoiwhduqwdqwuhd=", 71313))
                    .Change("fileC.txt", CFile("aihdqhdqudqdiuwqhdquwdqhdi=", 8193))
            );
    return 0;
}

While debugging, everything gets inserted into the map of root as intended.

Only problem that remains is the memory of the map is lost. Where should I free this memory, if it does not get freed implicitly?

TheBlue22
  • 39
  • 2
  • Your virtual copy constructor is not a copy constructor. I've also never heard of the necessity for a virtual copy constructor. – sweenish Apr 10 '21 at 12:30
  • Why `map> * systemMap;` and `new`? Why not just `map> systemMap;` without `new`? The `new` causes you trouble as you 1st) have no matching `delete` (memory leak) and 2nd) the default copy constructor (generated by compiler) will cause shared usage of the allocated `systemMap` in two instances which will make it impossible to you to `delete` it properly without a dangling pointer in a possible copy. – Scheff's Cat Apr 10 '21 at 12:34
  • @sweenish [SO: C++11 virtual copy constructor](https://stackoverflow.com/a/16794105/7478597) – Scheff's Cat Apr 10 '21 at 12:38
  • read this carefully, https://en.cppreference.com/w/cpp/language/rule_of_three and then probably go for the 0 option – Alessandro Teruzzi Apr 10 '21 at 12:38
  • @Scheff I appreciate the link, but the answer just kind of agrees with me and doesn't discuss an actual virtual copy constructor. If I have a pointer to `Base` only, and de-reference it to make a copy, properly implemented copy constructors (non-virtual) will do the job just fine. – sweenish Apr 10 '21 at 12:43
  • 1
    Answer this: when you `deepClone` a `CDirectory`, should the two share the same `systemMap`, or each get their own distinct copy of `systemMap`? If you later call `Change` on the original, should the clone also get changed? If the former, then just hold `systemMap` by `std::shared_ptr` instead of raw pointer, and your code would work as is. If the latter, then you need to provide a copy constructor for `CDirectory` that would make a deep clone of `systemMap` - allocate a new map, and insert into it copies of the keys and deep-cloned values from the original map. – Igor Tandetnik Apr 10 '21 at 13:01
  • @Scheff I have tried using map without new and with a default constructor for CDirectory, but when I try to compile it, I get an error: use of deleted function. – TheBlue22 Apr 10 '21 at 13:04
  • I think I realized what I am doing wrong here but I am not sure how I should fix it. When the destructor of the root is called, all the files/directories that it has are deallocated, but in case there is a directory, the files/directories inside the map of the direcotry are not. – TheBlue22 Apr 10 '21 at 13:27

0 Answers0