1

I am trying to increase the size of my two dynamically allocated array of pointers by one, so I'm creating temporary arrays, copying over my old values, deleting the originals, and then re-assigning them. The code compiles and runs fine, but I'm getting a memory leak and can't figure it out.

My member variables are width, *heights, and **data.

void Tetris::add_right_column() {

    width += 1;
    int* temp_heights = new int[width];
    char** temp_data = new char*[width];

    // copies old values into the temp one
    for (int i=0; i<width-1; i++) {
        temp_heights[i] = heights[i];
        temp_data[i] = data[i];
    }

    // adds new value into temps
    temp_data[width-1] = new char[0];
    temp_heights[width-1] = 0;

    // deletes original arrays of pointers
    delete [] heights;
    delete [] data;

    // reassigns the pointers
    heights = temp_heights;
    data = temp_data;

}

I'm using Dr. Memory and have some leaks:

Error #2: LEAK 28 direct bytes 0x019685c0-0x019685dc + 0 indirect bytes
# 0 replace_operator_new_array               [d:\drmemory_package\common\alloc_replace.c:2928]
# 1 Tetris::add_right_column                 [********/tetris.cpp:308]
# 2 additional_student_tests                 [********/main.cpp:276]
# 3 main                                     [********/main.cpp:41]

Error #3: LEAK 28 direct bytes 0x01968600-0x0196861c + 12 indirect bytes
# 0 replace_operator_new_array               [d:\drmemory_package\common\alloc_replace.c:2928]
# 1 Tetris::add_right_column                 [********/tetris.cpp:309]
# 2 additional_student_tests                 [********/main.cpp:276]
# 3 main                                     [********/main.cpp:41]

All I know is that the memory I'm allocating at these two lines:

int* temp_heights = new int[width];
char** temp_data = new char*[width];

is not being freed properly. I've tried:

delete [] temp_heights;
delete [] temp_data;

and

// after copying data[i] over to temp_data[i]
for (int i=0; i<width; i++) {
    delete data[i];
}

but both of these cause it to stop working. I know I can't use reallocate because I'm using new. What's the best way for me to avoid a memory leak in this case?

EDIT:

My constructor: 

Tetris::Tetris(int given_width) {
    width = given_width;
    heights = new int[width]; 
    // sets n (width) number of heights = to 0
    for (int i =0; i<width; i++) {
        heights[i] = 0;
    }
    // same type as the type of the arrays it points to
    data = new char*[width];
    for (int i=0; i<width; i++) {
        data[i] = new char[heights[i]];
    }
}

My destructor:

void Tetris::destroy() {
    delete [] data;
    delete [] heights;
 }

My class:

class Tetris {
public:

    // CONSTRUCTORS
    Tetris(int given_width);

    // ACCESSORS
    int get_width() const { return width;};
    int get_max_height() const;
    int count_squares() const;
    void print() const;
    void destroy();
    int count_squares();

    // MODIFIERS
    void add_piece(char piece, int rotation, int position);
    int remove_full_rows();
    void add_left_column() { };
    void add_right_column();
    void remove_right_column() { };
    void remove_left_column() { };

private:
    // MEMBER VARIABLES
    int width;
    int *heights;
    char **data;
};

Example of main:

void main() {
    Tetris tetris(6);
    tetris.print();
    tetris.add_right_column();
    tetris.print();
}
Sunden
  • 843
  • 3
  • 11
  • 24
  • *Copying a Dynamically Allocated Array over to a larger array without a Memory Leak* -- Simple -- use `std::vector` -- that is what it was designed for. – PaulMcKenzie Feb 19 '16 at 22:12
  • This is homework and I'm not allowed to use vectors or I most definitely would be. – Sunden Feb 19 '16 at 22:13
  • Well, have you thought about just creating a dynamic array class, and then using it instead of doing one-off stuff like this? Creating a working dynamic array class, and then using it in future assignments is a much better approach. – PaulMcKenzie Feb 19 '16 at 22:16
  • how does destructor looks? add_right_column() looks correct. – Vishal Kumar Feb 19 '16 at 22:21
  • @Sunden Does your `Tetris` class have the appropriate user-defined copy constructor, assignment operator, and destructor? If copies of `Tetris` objects are done, and you do not have these functions (or they are buggy), then the program will exhibit undefined behavior. – PaulMcKenzie Feb 19 '16 at 22:25
  • It would also help if you posted the test program, not just code that by itself, shows no problem. It is the code you're not showing us (or the code you didn't write, like the copy constructor, assignment operator and destructor) that's causing the issue. – PaulMcKenzie Feb 19 '16 at 22:32
  • 1
    This code looks correct to me. The only thing I would guess you are missing is in the destructor, you will need to delete all the elements of data[] before you delete the data array itself. Without the whole code, that's a wild guess though – Signal Eleven Feb 19 '16 at 22:42
  • I apologize, I posted the rest of my code above. – Sunden Feb 19 '16 at 22:58
  • Your class does not have a destructor. A destructor is not a function called `destroy`. Where is `~Tetris{}`? – PaulMcKenzie Feb 19 '16 at 23:05
  • 1
    Also, if you look at how you construct `Tetris`, you're making 3 calls to `new[ ]` (if you are not counting loops). Therefore you should be making 3 calls to `delete[ ]`, not just 2 calls when releasing the memory. – PaulMcKenzie Feb 19 '16 at 23:12
  • Thank you I tried deleting all of the data[w] that I assigned in the constructor and fixed the leak. I'll look into destructors. – Sunden Feb 19 '16 at 23:16

1 Answers1

1

Your data[] contains array of char* which you are allocating in the heap every time..

Before you delete [] data make sure you delete all the "new chars" pointed by your data array.

Sam Daniel
  • 1,800
  • 12
  • 22