0

I have a function that should delete the elements in a dynamic array of pointers.

I have built a function that dynamically creates this array which seems to be working fine (please let me know if you see any issues).

However, my deletion function will not let me delete the value at student[i], spcifically line 137. The code compiles fine with this line commented out, but the value needs to be deleted or else it will cause a memory leak.

Why can't I call delete student[i] like I am attempting at line 137? What is the issue? I am trying to learn and would appreciate a simplified answer.

Here is my code:

#include <iostream>
#include <cstring>
#include <string> //TESTING
#include <fstream> //needed to use files (contains definitions for ifstream and ofstream)
#include <stdlib.h>  //for exit 
#include <cctype> //char handling functions
#include <iomanip> //setprecision ,etc
#include <cstddef>

using namespace std;

const int SIZE = 101; //max size of arrays 

struct Student
{
   char *     name;
   float      gpa;
};

Student ** createStudentList(char ** names, int size);
bool destroyStudentList(Student ** studentList, int size);

int main ()
{
        Student ** studentList;

        char ** names;
        int size = 0;

        names = new char*[3];
        names[size] = new char[strlen("Lindsay")+1];
        strcpy(names[size], "Lindsay");
        size++;
        names[size] = new char[strlen("Emily") +1 ];
        strcpy(names[size], "Emily");
        size++;

        studentList = createStudentList(names, size);
        cout << "//TESTING: before destroyStudentList()" << endl;
        destroyStudentList(studentList, size);

        return 0;
}


//  The function creates an array of pointers to Student objects dynamically. 
//It allocates Student objects and set their name as the passed in “names” and gpa as 0. “size” is the number of “names” available.
// You can allocate 2*size as many pointers to Student objects just in case you need to add more to the list. 
//For the extra spots in the array, set them to nullptr. The function returns the pointer to the Student pointer array.
Student ** createStudentList(char ** names, int size)
{
        // code adapted from CS162 module 8 discussion post  for Lab 6 (Nancy Chan and Li Liang)

        int double_size = size *2;

        Student ** studentList = new Student * [double_size];

        Student * studentPtr = new Student [double_size];


        for (int i = 0; i < 2 * size; i++)
        {
                studentList[i] = &studentPtr[i];
        }


        for (int i = 0; i < size; i++)
        {
                studentList[i]->name = new char[strlen(names[i]) + 1];
                strcpy(studentList[i]->name, names[i]);
                studentList[i]->gpa = 0;

        }

        for (int i = size; i < double_size; i++)
        {
                studentList[i] = NULL;
        }

        return studentList;

}


//  The function deletes all the Student objects in the array along with their dynamically allocated data members, 
// such as name. It also releases the array of the pointers to the Student objects.  
// The function returns true if the operation is successful and false if “studentList” contains nullptr.
bool destroyStudentList(Student * studentList[], int size)
{
        int double_size = size *2;

         // When the pointer is nullptr, there is nothing to delete
        if (studentList == nullptr)
        {
                return false;
        }
        else
        {
                //cout << "//TESTING: in destroyStudentList. Else..." << endl;
                for (int i = 0; i < double_size; i++)
                {
                        if (studentList[i] != NULL) {
                        cout << (*studentList[i]).name << endl;
                        delete [] (*studentList[i]).name;/////can't delete the char array here
                        cout << "//TESTING1: delete [] studentList[i]->name;" << endl;

                        (*studentList[i]).name = NULL;
                        cout << "//TESTING2: studentList[i]->name = NULL;" << endl;

                        delete studentList[i];////////////WHY DOES THIS CAUSE AN EXCEPTION?
                        //cout << "//TESTING3: delete studentList[i];" << endl;
                        //
                        //studentList[i] = NULL;
                        //cout << "//TESTING4: studentList[i] = NULL;" << endl;
                        }
                }
        }
        delete studentList;
        studentList = nullptr;
        return true;

}
Improve this question
5
  • 1
    What is wrong in just using std::vector? And std::string instead of char*? Commented Mar 2, 2015 at 3:25
  • thank you for your input, but I am trying to understand the current problem with my current code. It's a learning exercise and if I change it without understanading the problem I will learn nothing. Commented Mar 2, 2015 at 3:29
  • @FluffyKittens If you're going to write code like this, it would be more worthwhile to write your own string class, and then use it. Then write your own vector class, and then use it. Having allocation and deallocation strewn about a program like this is not a good way to learn these things. Commented Mar 2, 2015 at 3:33
  • 1
    Why must schools torture their students like this and call it "c++" Commented Mar 2, 2015 at 4:05
  • I like to call this language c++-- Commented Mar 2, 2015 at 4:47

2 Answers 2

Reset to default
1

You asked:

delete studentList[i];////////////WHY DOES THIS CAUSE AN EXCEPTION?

Before I answer why that line is bad, let me start with a simple example. If you allocate memory with:

int* ip = new int[10];

The only valid way to deallocate the memory is to use:

delete [] ip;

The following are all invalid ways to trying to deallocate that memory.

delete ip;
delete [] ip+2;
delete [] ip+4;
delete ip+2;
delete ip+4;
delete &ip[2];
delete &ip[4];

The line

delete studentList[i];

suffers from that problem. Not only are you not using the array form of the delete operator but you are calling delete on individual elements from the array that was allocated using one call.

In createStudentList, you allocated memory for the array of Students using:

 Student * studentPtr = new Student [double_size];

and then assigned those pointers to studentList using:

   for (int i = 0; i < 2 * size; i++)
   {
      studentList[i] = &studentPtr[i];
   }

The only valid way to delete the array of Students is to use:

delete [] studentList[0];
Improve this answer
Sign up to request clarification or add additional context in comments.

1 Comment

Thank you. I thought about it overnight and this is what I expected. I thought that perhaps since my array is an array of pointers, that I had to de-allocate the memory at each index, because each index is a pointer that was allocated using new.
1

Your main bug is in the line:

delete studentList[i]; ////////////WHY DOES THIS CAUSE AN EXCEPTION?

It causes an exception because studentList[i] was not allocated via new.

In fact, it points partway into a block which was allocated via new[]. You can only delete a pointer that was allocated via new; you cannot delete parts of a block.

Your code does two Student allocations:

Student ** studentList = new Student * [double_size];
Student * studentPtr = new Student [double_size];

Therefore you must have exactly two deletes to free that memory (and not a delete in a loop).

In fact your create function never saved the value of studentPtr explicitly. But you can retrieve it by writing delete[] studentList[0];, because the create function stored studentPtr into studentList[0]. This will delete the entire block of students, and should be placed after the end of the for loop.

The second level of indirection in studentList is completely redundant; you can achieve all your goals just by using Student * and the size variable. It would simplify your code to get rid of all the junk with the second list of pointers.

The names block is way more complicated than it should be. Compare what you've got with this:

char const *names[] = { "Lindsay", "Emily" };
studentList = createStudentList(names, 2);

Since your create function is taking copies of names there is no need to use dynamic allocation for names.

(You will need to add const to the start of the first parameter for createStudentList , which it should have anyway).

If your class rules do not prevent you doing this, then changing Student's member char *name; to be std::string name; will save you a lot of trouble. Then you do not need the deletion loop at all.

It is very bad style for a class to have a pointer which owns some text, but expect the user of the class to do the memory management. Classes should have self-contained memory management. You could also achieve this by writing a constructor and destructor for Student, although you would also need to observe the Rule of Three. In fact you would basically be reinventing std::string.

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.