Copy constractor with memory allocation and deallocation

Question

I have written following piece of code

#include<iostream>
#include<cstring>
using namespace std;

///Driver class
class driver
{
    char *name;
    int age;
public:
    //Default contructor
    driver(){}

    //Constructor for initialize
    driver(int a, char* n)
    {
        age = a;
        int len = strlen(n);
        //Allocate memory for name
        name = new char[len];
        strcpy(name, n);
    }

    //Copy constructor
    driver(const driver &d)
    {
        name = new char[strlen(d.name)];
        strcpy(name, d.name);
        age = d.age;
    }

    void print()
    {
        cout<<"Name: "<<name<<endl;
        cout<<"Age: "<<age<<endl;
    }

    ~driver()
    {
        if(name != NULL)
        {
            delete name;
        }
    }
};

class automobile
{
    driver drv;
    char* make;
    int year;

public:
    automobile(driver d, char* m, int y)
    {
        drv = d;
        int len = strlen(m);
        make = new char[len];
        strcpy(make, m);
        year = y;
    }

    void print()
    {
        drv.print();
        cout<<"Make: "<<make<<endl;
        cout<<"Year: "<<year<<endl;
    }
    ~automobile()
    {
        if(make!=NULL)
        {
            delete[] make;
        }
    }
};

int main()
{
    driver d(15, "Jakir");
    automobile a(d, "Toyta", 1980);
    a.print();
    return 0;
}

I have to use char* not string and allocate memory dynamically. But when I run the code, there occurs an error of memory leaks. I think it's due to the copy constructor and de-allocation memory of driver class. How to fix the error? Any suggestion is appreciated. Thanks in advance.

Answer 1

There is a lot of stuff wrong with this code, but I will list a few of the big ones.

1. As smentioned by others, you new char's need to be one bigger than they are.
2. You default constructor should set name to nullptr.
3. you have a memory handling error whenever you do an assignment, eg on this line: drv = d; because it calls the default operator=, which is incorrect in your case. Ther eis the Law of the Big Three which loosely states that whenever you need either a (non-trivial) copy constructor, copy assignment operator, or destructor, you'll most likely need to implement the others, too. You need to write an operator=!

Based on existing code, I would expect your operator= to look vaguely like this:

//assignment operator
const driver&operator=(const driver &rhs)
{
    if (this==&rhs) return *this;
    delete[] this->name;
    this->name = new char[strlen(rhs.name)+1];
    strcpy(this->name, rhs.name);
    this->age = rhs.age;
    return *this;
}

Do all that, and your core dump goes away.

PS Please, please just learn to use std::strings, new-ing char arrays and managing the memory yourself is a bad bad move.

Answer 2

name = new char[len+1]; not name = new char[len];

name = new char[strlen(d.name) + 1]; not name = new char[strlen(d.name)];

delete[] not delete

Need to define an assignment operator driver& operator=(const driver &d) in order to follow the rule of three.

Similar changes need to automobile.

However I don't see a memory leak, what makes you think you have one?

Answer 3

In main method you need to do like this,

driver* d = new driver(15, "Jakir");
automobile* a = new automobile (d, "Toyta", 1980);
a->print();
if ( a ) delete a ;
if ( d ) delete d ;
return 0 ;