2

I'm currently creating a FAT filesystem in C++. I have three classes:

  • Sdisk (which formats the disk via string to file output).
  • Filesys (which writes the FAT and the root).
  • Shell (offers ability to perform various file system operations.)

During each class coding I wrote simple main initializations of Sdisk and Filesys constructors. Now that I'm writing the Shell when I test Shell's default constructor I'm having issues with memory. Specifically: EXC_BAD_ACCESS(code=1 address=0xfffffffffffffff....).

I decided to use my debugger to find out what went wrong and determined that the values I was passing in the Shell constructor to the Sdisk default constructor wasn't correct. I started to dig and found out that calling another classes constructor inside of the current classes constructor was called: "Delegating" or "Constructor Chaining". I had no idea. I found out that you have to "use it from the initialization list, not from the constructor body".

I've created an extra constructor with no parameters for Shell. What I need to do is call Sdisk from inside of the Shell constructor with Sdisk's default three parameters. But when I try that I continue to get the memory errors. I've even attempted to give the default constructor of Shell no parameters but as long as I call it in main it causes the error.

Any help regarding this matter is extremely appreciated! Thank you!

Here is my reference for Delegating Constructors

class Shell: public Filesys
{
    public:    
    Shell(string filename, int blocksize, int numberofblocks):Filesys(disk) {Filesys(filename,blocksize,numberofblocks);};                // creates the file system.
        int dir();              // call ls which lists all files
        int add(string file);   // add a new file using input from the keyboard.
        int del(string file);   // deletes the file
        int type(string file);  //lists the contents of file
        int copy(string file1, string file2);//copies file1 to file2
    friend class Sdisk;
    friend class Filesys;
};



class Sdisk
{
    public :
    Sdisk() { }

    Sdisk(string diskname);                                         // Default Constructor
    Sdisk(string diskname, int numberofblocks, int blocksize);
    int getblock(int blocknumber, string& buffer);
    int putblock(int blocknumber, string buffer);
    int getblocksize() {return blocksize; }                         // Returns the blocksize.
    int getnumberofblocks() { return numberofblocks; }              // Returns the number of blocks.
    string getfilename() { return diskname; }                       // Returns the disk name.
    friend class Shell;
    friend class Filesys;

    private :

    int numberofblocks;                                             // number of blocks on disk
    string diskname;                                                // file name of pseudo-disk
    string diskname1;
    int blocksize;                                                  // block size in bytes/the number of blocks.
};

  class Filesys
{
public:

    Filesys(Sdisk&);
    int fsclose();
    int newfile(string file);
    int rmfile(string file);
    int getfirstblock(string file);
    int addblock(string file, string block);
    int delblock(string file, int blocknumber);
    int readblock(string file, int blocknumber, string& buffer);
    int writeblock(string file, int blocknumber, string buffer);
    int nextblock(string file, int blocknumber);
    bool checkblock(string file, int blocknumber);
    vector<string> block(string buffer, int b);
    Sdisk disk;
    friend class Shell;

    private :

    int fssync();                   //writes the Root and FAT to the disk.
    string buffer;
    int rootsize;                   // maximum number of entries in ROOT
    int fatsize;                    // number of blocks occupied by FAT
    vector<string> filename;        // filenames in ROOT
    vector<int> firstblock;         // firstblocks in ROOT parallel
    vector<int> fat;                // FAT # of blocks
};

int main()
{   
    Shell("disk",256,128);
    string s;
    string command="go";
    string op1,op2;

    while (command != "quit")
    {
        command.clear();
        op1.clear();
        op2.clear();
        cout << "$";
        getline(cin,s);
        unsigned long firstblank = s.find(' ');
        if (firstblank < s.length()) s[firstblank]='#';
        unsigned long secondblank = s.find(' ');
        command=s.substr(0,firstblank);
        if (firstblank < s.length())
            op1=s.substr(firstblank+1,secondblank-firstblank-1);
        if (secondblank < s.length())
            op2=s.substr(secondblank+1);
        if (command=="dir")
        {
            cout << "dir" << endl;

            // use the ls function

        }
        if (command=="add")
        {
            // The variable op1 is the new file
            cout << "add" << endl;

        }
        if (command=="del")
        {
            cout << "del" << endl;
            // The variable op1 is the file
        }
        if (command=="type")
        {
            cout << "type" << endl;
            // The variable op1 is the file
        }
        if (command=="copy")
        {
            cout << "copy" << endl;
            // The variable op1 is the source file and the variable op2 is the destination file.
        }
        if (command=="exit")
        {
            cout << "Exiting now..." << endl;
            return 0;
        }

    }

    return 0;
}





Filesys::Filesys(Sdisk& sdisk):Sdisk(disk)
{
    this-> disk = sdisk;
    rootsize = disk.getblocksize()/12;
    fatsize = (disk.getnumberofblocks()*5) / (disk.getblocksize())+1;
    cout << "rootsize: " << rootsize << endl << "fatsize: " << fatsize << endl << "number of blocks: " <<  disk.getnumberofblocks() << endl << "getblocksize(): " << disk.getblocksize() << endl;

    for(int i=0; i<rootsize; i++)
    {
        filename.push_back("XXXXXX");
        firstblock.push_back(0);
    }

    int k= disk.getnumberofblocks();
    fat.push_back(fatsize + 2);

    for (int i = 0; i <= fatsize; i++)
    {
        fat.push_back(0);

    }

    for(int i = fatsize + 2; i < k; i++)

    {
        fat.push_back(i+1);
    }
    fat[fat.size()-1] = 0;
    fssync();
}


Sdisk::Sdisk(string disk)
{
    diskname = disk + ".dat";
    diskname1 = disk + ".spc";
    ifstream ifile(diskname1.c_str());

    if(ifile.is_open())
    {
        ifile >> numberofblocks >> blocksize;
        ifile.close();
    }

    else
    {
        cout << "Was unable to open the file" << endl;
    }
}

// Sdisk default constructor
Sdisk::Sdisk(string disk, int numberofblocks, int blocksize)
{
    this->diskname = disk + ".dat";
    this->diskname1 = disk + ".spc";
    this->numberofblocks = numberofblocks;
    this->blocksize = blocksize;
    fstream spcfile;
    fstream datfile;
    spcfile.open((this->diskname1).c_str(),ios::in | ios::out);
    datfile.open((this->diskname).c_str(),ios::in | ios::out);

    if (spcfile.good() && datfile.good())
    {
        cout << "The disk named: " << diskname.c_str() << " exists and is now ready to be written to." << endl;
    }
    else // .spc/.dat file creation.
    {
        cout << "The disk: " << diskname.c_str() << "could not be found. " << endl;
        cout << "Both the SPC and DAT file were not found. Creating both now. Please wait...." << endl;
        spcfile.open((this->diskname1).c_str(),ios::out);
        datfile.open((this->diskname).c_str(),ios::out);
        spcfile << numberofblocks << " " << blocksize;
        cout << "The SPC file " << diskname.c_str() << " was created" << endl;
        cout << "The DAT file " << diskname.c_str() << " was created" << endl;

        for (int i=0; i<numberofblocks*blocksize; i++)
        {
            datfile.put('#');           // Fills the file with '#' character.
        }
    }
    spcfile.close();
    datfile.close();
    return;
}
SaundersB
  • 607
  • 2
  • 12
  • 25
  • Well certainly this is very bad `Shell(string filename, int blocksize, int numberofblocks):Filesys(disk) {Shell(filename,blocksize,numberofblocks);};`. But I can't figure what you wanted to do here. It doesn't make sense even if it was proper syntax for delegation it looks like an infinite recursion. – luk32 May 16 '15 at 20:05
  • I was thinking of inheriting Sdisk with Shell instead of Filesys. Then in the Filesys constructor do a check and call Sdisk. Problem is when I go to inherit Sdisk into FIlesys it doesn't state it is a class. I #include"Sdisk.h" and everything I'm just not sure what's the best choice. – SaundersB May 16 '15 at 20:32

2 Answers2

1

The problem is here:

Shell(string filename, int blocksize, int numberofblocks) : Filesys(disk) 
{
   Shell(filename,blocksize,numberofblocks);
};

The Shell constructor creates in the body a temporary shell object, which itself calls the Shell constructor again and so on. So you will end up with an infinite recursion and a full stack.

Other remarks:

  • In your shell constructor you also initalize the base class subobject with the mem-initializer Filesys(disk). I couldn't find a valid disk in your code snippet. But as you experience runtime problems and not compilation errors, I suppose it was simply lost in the copy&paste.

  • Are you sure that Shell should inherit from Filesys ? I.e. can you say that your Shell is a FAT filesystem ? And how would your class design evolve if later you'd decide to enrich the filesystems that your Shell suports with say NTFS or EXT3 filesystems ?

  • Finally, is the number of blocks and the blocksize not parameters of the filesystem rather than the shell you construct on its top ?

For these reasons I'd rather go for something like:

class Shell
{
    string fname; 
    Filesys &fs;  
public:    
    Shell(string filename, Filesys &filesystem) 
        : fname(filename), fs(disk)
    { ... };                // creates the file system.
    ...
};

In this case you'd create the Shell:

Sdisk mydisk("drive1", 32768, 4096);  // disk data for the disk constructor
Filesys mysystem(mydisk);             // fs parameters for the fs
Shell myshell("A:", mysystem);        // higher level abstraction 
Christophe
  • 68,716
  • 7
  • 72
  • 138
  • You're right. I removed that and instead am trying to figure out a way to pass those three parameters to Filesys, which will in turn pass them to Sdisk. I want to call the Shell, make the disk with Sdisk, and then build the file system with Filesys. I don't know how to inherit two classes with a single class though. – SaundersB May 16 '15 at 20:20
  • You probably don't want inheritance. Your `Shell` object probably just needs a reference or pointer to a `Filesys` object. Or maybe even a collection of `Filesys` objects. – Michael Burr May 16 '15 at 20:28
0

I hope I got it right. You got quite many problems in your code. As I said in the comment this is very bad:

Shell(string filename, int blocksize, int numberofblocks):Filesys(disk) {
   Shell(filename,blocksize,numberofblocks);
};

It is an obvious infinite recursion. You said you needed to call Sdisk from inside the Shell so I guess you meant:

Shell(string filename, int blocksize, int numberofblocks):Filesys(disk) {
   Sdisk(filename,blocksize,numberofblocks);
};

However, it won't help you in any way. Such syntax will create a temporary Sdisk which will die immediately. You said you want to use default parameter but you don't provide any anywhere.

Generally your design is quite bad. The relations between classes are not clear and you ended up with strange solutions which do not make much sense. I.e. why do you want to call this constructor? What is the relation between Sdisk and Shell? Shell doesn't have any member variables so it cannot hold any state (I'm almost certain that it's not intended)... etc.

luk32
  • 15,812
  • 38
  • 62
  • I see. I corrected the infinite recursion but haven't remedied the issue of temporary values being used. I'm supposed to call Sdisk to build the disk from Shell and then call Filesys in Sdisk to build the file system. The shell's default constructor is supposed to build the file sytem by calling Sdisk or Filesys. I'm not sure which. Then the shell have various functions to operate on the file system. Shell inherits Filesys because it uses various functions. But I don't see why we don't inherit Sdisk instead. I can't do that because I'd have to inherit two classes in Sdisk. – SaundersB May 16 '15 at 20:26
  • Also the default parameters are in my main: Shell("disk",256,128); – SaundersB May 16 '15 at 20:27