Thread: Is this a memory leak?

  1. #1
    Registered User
    Join Date
    Mar 2016
    Posts
    110

    Is this a memory leak?

    Code:
    #include <iostream>
    #include <string>
    #include <stdlib.h>
    #include <string.h>
    
    
    using namespace std;
    
    
    class Student{
    private:
        char* name;
        int* marks;
    
    
    public:
        Student(char * n, int* a){
            name = new char[10]();
            marks = new int[3]();
            name = n;
            marks = a;
        }
    
    
        ~Student(){
            cout << "Destructor called" << endl;
            delete name;
            delete[] marks;
        }
    
    
        void display(){
            cout << "Name          : " << name << endl;
            cout << "Subject marks : " << endl;
            for(int i=0; i<3; i++) {
                cout << marks[i] << endl;
            }
        }
    };
    
    
    int main(){
        // Name
        char* name = (char*) malloc(sizeof(char)*10);
        strcpy(name,"Alan");
    
    
        // Marks
        int* marks = new int[3];
        marks[0] = 23;
        marks[1] = 20;
        marks[2] = 40;
    
    
        Student *s1 = new Student(name, marks);
        s1->display();
    
    
        delete s1;
    
    
    
    
        return 0;
    }
    I saw this code posted on what seems to be a reputable blog. I noticed line 20 and 21 looks like a memory leak. Is it? I actually have another question but first want to clarify this point.

  2. #2
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    38,163
    Yes, there are multiple memory leaks.

    Code:
    class Student{
    private:
        char* name;
        int* marks;
     
     
    public:
        Student(char * n, int* a){
            name = new char[10]();
            marks = new int[3]();
            //!! Both the above allocations are leaks, since you just overwrite your pointers.
            //!! Also, having a pointer to something on the outside is VERY bad style.
            //!! You have no control over how it was allocated (if at all), and no control
            //!! over it's lifecycle.  It could disappear without warning, leaving you with a 
            //!! stale (or dangling) pointer.
            name = n;
            marks = a;
        }
     
     
        ~Student(){
            cout << "Destructor called" << endl;
            delete name;
            //!! name was allocated with [], so you need to use delete[]
            //!! well, you would, if you fixed your ctor to do the right thing and make
            //!! copies of the inputs.
            //!! As is, it's just plain BROKEN because name was created with malloc
            //!! and not new.
            delete[] marks;
        }
     
     
        void display(){
            cout << "Name          : " << name << endl;
            cout << "Subject marks : " << endl;
            for(int i=0; i<3; i++) {
                cout << marks[i] << endl;
            }
        }
    };
     
     
    int main(){
        // Name
        //!! Don't use malloc to allocate memory in C++ programs
        //!! malloc doesn't cause the ctor to be called.
        //!! Whilst technically, it's OK for a PoD type, it's not a habit
        //!! you want to cultivate.
        char* name = (char*) malloc(sizeof(char)*10);
        strcpy(name,"Alan");
     
        // Marks
        int* marks = new int[3];
        marks[0] = 23;
        marks[1] = 20;
        marks[2] = 40;
     
        Student *s1 = new Student(name, marks);
        s1->display();
     
        delete s1;
    
        return 0;
    }
    > I saw this code posted on what seems to be a reputable blog
    Please tell us, so we can go and have a good laugh...
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

  3. #3
    Registered User
    Join Date
    Mar 2016
    Posts
    110
    Ok, but is line 20 and 21 specifically a memory leak? I dont see comments next to them in your comments. My rationale is as follows:
    Line 20: name = n; // name USED to point to an area of heap memory and now it points to what n points to. Therefore the area it used to point to is lost
    Line 21: marks = a; // same as above

    As requested, the article is from Medium:
    https://medium.com/tech-vision/all-about-destructors-in-c-62b5f534057e

  4. #4
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    38,163
    > Ok, but is line 20 and 21 specifically a memory leak?
    Well they become memory leaks through the assignments which follow on the next two lines.

    Do you know the difference between these two statements?
    name = n;
    vs
    strcpy(name,n);

    Which preserves the value of name (ie, the value returned by calling new).
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

  5. #5
    Registered User
    Join Date
    Oct 2018
    Posts
    18
    A bit unusual description of the problem)))
    If I doubt about memory leaks, I can check the code with Valgrind (for Linux) or Deleaker or any other memory leak detection tool for Windows.
    Is this code from cplusplus?

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. memory leak
    By aruna1 in forum C++ Programming
    Replies: 3
    Last Post: 08-17-2008, 10:28 PM
  2. Memory Leak
    By jtullo in forum C Programming
    Replies: 7
    Last Post: 12-11-2006, 11:45 PM
  3. Replies: 2
    Last Post: 09-28-2006, 01:06 PM
  4. Do I have a memory leak? (SDL)
    By antex in forum Game Programming
    Replies: 3
    Last Post: 03-02-2006, 06:18 PM
  5. Memory Leak or not?
    By Eber Kain in forum C++ Programming
    Replies: 4
    Last Post: 11-21-2001, 01:05 PM

Tags for this Thread