C++ memory access error

Question

I've got a problem when trying to call a function Organization::addWorker, it seems to fail on a memory deallocation step, an operator delete[], but the memory have already been freed which the pointer shows to.

call stack:

lab1.exe!Person::setName(char * thename)Line 80 C++

lab1.exe!Coworker::operator=(Coworker & theco)Line 303 C++

lab1.exe!Organization::addWorker(Coworker & theworker)Line 404 C++

lab1.exe!main()Line 97 C++

   #include <string>
    #include "class_Date.h"
    
    using namespace std;
    
    enum workNec { worker, manager, employer };
    
    class Person {
    
        char* name; 
        char* surname;
        Date bd;
    
    public:
    
        Person();
        Person(char* thename, char* thesurname, Date thebd);
        Person(const Person &theperson);
        ~Person();
    
        char* getName() { return name; }
        char* getSurname() { return surname; }
        Date getBD() { return bd; }
    
        void getFullInfo();
    
        Person & setName(char* thename);
        Person & setSurname(char* thesurname);
        Person & setBD(int dd, int mm, int yy);
        Person & setBD(Date thebd);
    
        Person & operator=(Person& thepers);
    
    };
    
    Person::Person() { 
    
        name = new char [1];
        strcpy(name, "");
        surname = new char[1];
        strcpy(surname, "");
    
    }
    
    Person::Person(char* thename, char* thesurname, Date thebd) { 
    
        name = new char [strlen(thename)+1];
        surname = new char [strlen(thesurname)+1];
        strcpy(name, thename);
        strcpy(surname, thesurname);
        bd = thebd;
    
    }
    Person::Person(const Person & theperson) { 
    
        name = new char[strlen(theperson.name) + 1];
        surname = new char[strlen(theperson.surname) + 1];
        strcpy(name, theperson.name);
        strcpy(surname, theperson.surname);
        bd = theperson.bd;
    
    }
    Person::~Person() {
    
        cout << "Destructing Person " << name << endl;
        delete [] name;
        delete [] surname;
    
    
    }
    
    void Person::getFullInfo() { 
        
        cout << name << " " << surname << " "; bd.getFullDate(); 
    
    }
    
    Person & Person::setName(char* thename)  { 
    
        delete [] name;
        name = new char[strlen(thename) + 1]; strcpy(name, thename); 
        return *this; 
    
    }
    
    Person & Person::setSurname(char* thesurname) { 
        
        delete [] surname;
        surname = new char[strlen(thesurname) + 1]; strcpy(surname, thesurname);
        return *this; 
    
    }
    Person & Person::setBD(int dd, int mm, int yy) { 
        
        bd.setDD(dd); bd.setMM(mm); bd.setYY(yy); 
        return *this; 
    
    }
    
    Person & Person::setBD(Date thebd) { 
    
        bd = thebd; 
        return *this; 
    
    }
    
    Person & Person::operator=(Person& thepers) {
    
        delete [] this->name;
        delete [] this->surname;
        this->name = new char[strlen(thepers.name) + 1];
        this->surname = new char[strlen(thepers.surname) + 1];
        strcpy(this->name, thepers.name);
        strcpy(this->surname, thepers.surname);
        bd = thepers.bd;
        return *this;
    }
    
    //*************************************************************************************************
    
    class Position {
    
        char* department;
        workNec wn;
        int salary;
    
    public:
    
        Position();
        Position(char *thedept, workNec thewn, int thesalary);
        Position(const Position & thepos);
        ~Position();
    
        char* getDept() { return department; }
        workNec getWorkNec() { return wn; }
        int getSalary() { return salary; }
    
        void getFullInfo();
    
        Position & setDept(char* dept);
        Position & setWorkNec(workNec nec);
        Position & setSalary(int sal);
    
    };
    
    
    Position::Position() {
    
        department = new char[1];
        strcpy(department, "");
    
    }
    
    Position::Position(char *thedept, workNec thewn, int thesalary) { 
    
        department = new char[strlen(thedept) + 1];
        strcpy(department, thedept);
        wn = thewn; 
        salary = thesalary; 
    
    }
    
    Position::Position(const Position & thepos) { 
    
        department = new char[strlen(thepos.department)+1]; 
        strcpy(department, thepos.department);
        wn = thepos.wn; 
        salary = thepos.salary; 
    
    }
    
    Position::~Position() { 
    
        cout << endl << "Deleting Position " << department << endl;
        delete [] department;
    
    }
    
    void Position::getFullInfo() { 
        
        cout << department << " " << wn << " " << salary << " "; 
    
    }
    
    Position & Position::setDept(char* dept) { 
    
        delete [] department;
        department = new char[strlen(dept)+1];
        strcpy(department, dept);
        return *this; 
    
    }
    Position & Position::setWorkNec(workNec nec) {
        
        wn = nec; 
        return *this; 
    
    }
    Position & Position::setSalary(int sal) { 
        
        salary = sal; 
        return *this; 
    
    }
    
    //*************************************************************************************************
    
    class Coworker {
    
        Person person;
        Position position;
    
    public:
    
        Coworker();
        Coworker(Person &theperson, Position &thepos);
        Coworker( Coworker & theco);
        ~Coworker();
    
        Person getPerson() { return person; }
        Position getPosition() { return position; }
    
        void getFullInfo();
    
        Coworker & setPerson(Person &per);
        Coworker & setPosition(Position &pos);
    
        Coworker & operator=(Coworker& theco);
        
    };
    
    Coworker::Coworker() {
    
    }
    
    Coworker::Coworker(Person &theperson, Position &thepos) { 
        
        person.setBD(theperson.getBD());
        person.setName(theperson.getName());
        person.setSurname(theperson.getSurname());
        position.setDept(thepos.getDept());
        position.setSalary(thepos.getSalary());
        position.setWorkNec(thepos.getWorkNec());
    
    }
    
    Coworker::Coworker(Coworker & theco) { 
    
        Person pers;
        pers = theco.person;
        Position pos = theco.position;
    
        person.setBD(pers.getBD());
        person.setName(pers.getName());
        person.setSurname(pers.getSurname());
        position.setDept(pos.getDept());
        position.setSalary(pos.getSalary());
        position.setWorkNec(pos.getWorkNec());
    
    }
    
    Coworker::~Coworker() {
    
    
        cout << endl << "Deleting Coworker " << endl;
    
    }
    
    void Coworker::getFullInfo() { 
        
        person.getFullInfo(); cout << " ";  position.getFullInfo(); 
    
    }
    
    Coworker & Coworker::setPerson(Person &per) { 
        
        Person tper = per;
        person.setBD(tper.getBD());
        person.setName(tper.getName());
        person.setSurname(tper.getSurname());
        return *this; 
    
    }
    Coworker & Coworker::setPosition(Position &pos) { 
        
        Position tpos = pos;
        position.setDept(tpos.getDept());
        position.setSalary(tpos.getSalary());
        position.setWorkNec(tpos.getWorkNec());
        return *this; 
    
    }
    
    Coworker & Coworker::operator=(Coworker& theco) {
    
        Person pers;
        pers = theco.person;
        Position pos;
        pos = theco.position;
    
        person.setBD(pers.getBD());
        person.setName(pers.getName());
        person.setSurname(pers.getSurname());
        position.setDept(pos.getDept());
        position.setSalary(pos.getSalary());
        position.setWorkNec(pos.getWorkNec());
        return *this;
    }
    
    class Organization {
    
        char* title;
        Coworker * coworker;
        int cwAmount;
    
    public:
    
        Organization();
        Organization(char* thetitle, Coworker * theco, int thecw);
        Organization(const Organization & org);
        ~Organization();
    
        void addWorker(Coworker &theworker);
    
        char* getTitle() { return title; }
        int getAmount() { return cwAmount; }
        Coworker * getCoworker() { return coworker; }
    
        Organization & setTitle(char* tit);
        Organization & setAmount(int am);
        Organization & setCoworker(Coworker *theco);
    
        void getFullInfo();
        void getShortInfo();
    
    };
    
    //*************************************************************************************************
    
    Organization::Organization() {
    
        title = new char[1];
        strcpy(title, "");
        cwAmount = 0;
        coworker = new Coworker[0];
    
    }
    
    
    Organization::Organization(char* thetitle, Coworker * theco, int thecw) { 
        
        title = new char[strlen(thetitle) + 1];
        strcpy(title, thetitle);
    
        cwAmount = thecw; 
        coworker = new Coworker[cwAmount];
    
        for (int i = 0; i < cwAmount; i++) {
    
            coworker[i].setPerson(theco[i].getPerson());
            coworker[i].setPosition(theco[i].getPosition());
    
        }
    
    }
    
    Organization::Organization(const Organization & org) { 
        
        title = new char[strlen(org.title) + 1];
        strcpy(title, org.title);
    
        cwAmount = org.cwAmount; 
    
        coworker = new Coworker[cwAmount];
    
        for (int i = 0; i < cwAmount; i++) {
    
            coworker[i].setPerson(org.coworker[i].getPerson());
            coworker[i].setPosition(org.coworker[i].getPosition());
    
        }
    
    }
    
    Organization::~Organization() { 
        
    
        cout << endl << "Deleting Organization " <<title<< endl;
        delete [] coworker;
    
    };
    
    void Organization::addWorker(Coworker &theworker) {
        cout << "a";
    
        Coworker * new_coworker = new Coworker[cwAmount + 1];
    
        for (int i = 0; i < cwAmount; i++) new_coworker[i] = coworker[i];
    
        cwAmount++;
    
        new_coworker[cwAmount] = theworker;
    
        delete [] coworker;
    
        coworker = new_coworker;
    
    }
    
    void Organization::getFullInfo() {
    
        std::cout << title << " " << cwAmount << endl;
        for (int i = 0; i < cwAmount; i++) {
            coworker[i].getPerson().getFullInfo();
            cout << " ";
            coworker[i].getPosition().getFullInfo();
            cout << endl;
        }
    
    }
    
    void Organization::getShortInfo() {
    
        int total = 0;
        for (int i = 0; i < cwAmount; i++) {
    
            total += coworker[i].getPosition().getSalary();
    
        }
    
        std::cout << this->title << " " << total << std::endl;
    
    }
    
    Organization & Organization::setTitle(char* tit) { 
        
        title = new char[strlen(tit)+1]; 
        strcpy(title, tit);
        return *this; 
    
    }
    
    Organization & Organization::setAmount(int am) { 
        
        cwAmount = am; 
        return *this; 
    }
    
    Organization & Organization::setCoworker(Coworker *theco) { 
    
        delete [] coworker;
        coworker = new Coworker [cwAmount];
    
        for (int i = 0; i < cwAmount; i++) {
    
            coworker[i].setPerson(theco[i].getPerson());
            coworker[i].setPosition(theco[i].getPosition());
    
        }
    
        return *this; 
    
    }

main.cpp:

#define _CRT_SECURE_NO_WARNINGS

#include "classes_v23.h"

using namespace std;

int main() {
    {
    Date d1;

    Date d2(19, 04, 1995);

    Date d3 = d2;

    d1.getFullDate(); cout << endl;
    d2.getFullDate(); cout << endl;
    d3.getFullDate(); cout << endl;

    //******************************

    Person pers1;

    pers1.setBD(d2);
    pers1.setName("Ihor");
    pers1.setSurname("Pukish");

    Person pers2("name2", "surname2", d2);
    Person pers3 = pers2;

    pers1.getFullInfo(); cout << endl;
    pers2.getFullInfo(); cout << endl;
    pers3.getFullInfo(); cout << endl;

    //******************************

    Position pos1;

    pos1.setDept("IASA");
    pos1.setSalary(100);
    pos1.setWorkNec(employer);

    Position pos2("dept2", worker, 200);
    Position pos3 = pos2;

    pos1.getFullInfo(); cout << endl;
    pos2.getFullInfo(); cout << endl;
    pos3.getFullInfo(); cout << endl;

    //******************************

    Coworker co1;
    co1.setPerson(pers1);
    co1.setPosition(pos1);

    Coworker co2(pers2, pos2);
    Coworker co3 = co2;
    Coworker co4(pers1, pos1);

    co1.getFullInfo(); cout << endl;
    co2.getFullInfo(); cout << endl;
    co3.getFullInfo(); cout << endl;

    //******************************

    Coworker * cow = new Coworker[3];
    cow[0] = co1;
    cow[1] = co2;
    cow[2] = co3;

    Organization org1("title", cow, 3);
    org1.addWorker(co4);
}
system("pause");
}

Answer 1

I believe the problem is:

void Organization::addWorker(Coworker &theworker) {
    cout << "a";

    Coworker * new_coworker = new Coworker[cwAmount + 1];

    for (int i = 0; i < cwAmount; i++) new_coworker[i] = coworker[i];

    cwAmount++;

    new_coworker[cwAmount] = theworker;

    delete [] coworker;

    coworker = new_coworker;

}

Notice that you increment cwAmount before assigning theworker. However I think you should assign first, then increment. The empty element is located at the pre-increment index.

This could also be done much easier using a vector.

Answer 2

First, you included <string>, but you failed to use what <string> has to offer, namely std::string. Since you say you must use C-style strings, the correct header is <cstring>, not <string>.

Second, this function will not work if NULL is passed, or even a bogus pointer:

Person::Person(char* thename, char* thesurname, Date thebd) { 
        name = new char [strlen(thename)+1];   // undefined behavior if NULL is passed
        surname = new char [strlen(thesurname)+1];  // undefined behavior if NULL is passed
        //...
    }

That function should be checking for NULL pointer argumemts. But even with that, there is a risk of being passed a pointer that isn't pointing to a valid buffer.

Now, as others stated, you need to write your assignment operator correctly and for every class that has pointers to dynamically allocated memory.

Let's start with the Person assignment operator:

 Person & Person::operator=(Person& thepers) {
        delete [] this->name;
        delete [] this->surname;
        this->name = new char[strlen(thepers.name) + 1];
        this->surname = new char[strlen(thepers.surname) + 1];
        strcpy(this->name, thepers.name);
        strcpy(this->surname, thepers.surname);
        bd = thepers.bd;
        return *this;
    }

Not only is this redundant (most of this code is in the copy constructor), it fails for the following reasons:

1. You delete[] the name and surname. What happens if the following new[] throws an exception? You've messed up your object by deleting your data before you could be assured that new wouldn't fail.
2. You don't check for self-assignment.
3. The item should be passed as a const reference.

The way to fix is simple, and I suggest you do that for all of your other classes.

 #include <algorithm>
 //...
 Person & Person::operator=(const Person& thepers) 
 {
      Person other(thepers);
      swap(*this, other);
      return *this;
 }

 void Person::swap(Person& left, Person& right)
 {
      std::swap(left.name, right.name);
      std::swap(left.surname, right.surname);
      std::swap(left.bd, right.bd);
  }

That's it, nothing else. This is called the copy/swap idiom. As long as you have coded a working and correct copy constructor, the code above not only copies correctly, it gets rid of the issues of self-assignment, and destroying members prematurely.

The way it works is to create a temporary, "steal" the temporary's stuff for your own by swapping yours with the temporary's , and have the temporary die, along with your old stuff. That in layman's terms is what is happening.

Please do this for the other classes. For example, Coworker:

Coworker & Coworker::operator=(const Coworker& theco) 
{
   Coworker temp(theco);
   swap(*this, temp);
   return *this;
}

void Coworker::swap(Coworker& left, Coworker& right)
{
    std::swap(left.person, right.person);
    std::swap(left.position, right.position);
}

In addition, your Organization class is missing an assignment operator. Just add one by using the above techniques.

Also, your setName function also suffers from deletion of memory before reallocation:

Person & Person::setName(char* thename)  { 
    delete [] name;
    name = new char[strlen(thename) + 1]; strcpy(name, thename); 
    return *this; 
}

This can also be alleviated by allocating to a temporary first, and then assigning the temporary at the end of the function:

Person & Person::setName(char* thename)  
{ 
    char *tempname = new char[strlen(thename) + 1]; 
    strcpy(tempname, thename); 
    delete [] name;
    name = tempname;
    return *this;
}