Memory leak with template for pointers and ints. What am i doing wrong?

Question

Getting to know C++, and trying to template a class to work with Ints and Pointers for a struct. The output is as expected, but testing with valgrind there seems to be a memory leek, from an unfreed memory.

I believe it has something to do with the way i declare the list variable in the class init.

What am I missing and how can i fix it? Thanks.

#include <stdio.h>

template <class T>
class List {
    T* list;

public:
    int length;

    List(int len) {
        list = new T[len];
        length = len;
    }

    virtual ~List() {
        delete[] list;
    }

    T get(int index) {
        return list[index];
    }

    void set(int index, T val) {
        list[index] = val;
    }
};
/*
    You shouldn't change the code below, unless you want to _temporarily_ change the main function while testing.
    Change it back when you're done.
*/
typedef struct Point_ {
    int x;
    int y;
} Point;

int main(){
    List<int> integers(10);
    for(int i = 0; i < integers.length; i++){
        integers.set(i, i * 100);
        printf("%d ", integers.get(i));
    }
    printf("\n"); // this loop should print: 0 100 200 300 400 500 600 700 800 900

    List<Point *> points(5);
    for(int i = 0; i < points.length; i++) {
        Point *p = new Point;
        p->x = i * 10;
        p->y = i * 100;
        points.set(i, p);
        printf("(%d, %d) ", points.get(i)->x, points.get(i)->y);
        delete p;
    }
    printf("\n"); // this loop should print: (0, 0) (10, 100) (20, 200) (30, 300) (40, 400)
}

compiled with g++ like this:

g++ -Wall p2_templates.cpp -o p2_templates

Used valgrind with this command:

valgrind --tool=memcheck ./p2_templates

Getting this result from valgrind:

==22396== Memcheck, a memory error detector
==22396== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==22396== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==22396== Command: ./p2_templates
==22396== 
0 100 200 300 400 500 600 700 800 900 
(0, 0) (10, 100) (20, 200) (30, 300) (40, 400) 
==22396== 
==22396== HEAP SUMMARY:
==22396==     in use at exit: 72,704 bytes in 1 blocks
==22396==   total heap usage: 9 allocs, 8 frees, 73,848 bytes allocated
==22396== 
==22396== LEAK SUMMARY:
==22396==    definitely lost: 0 bytes in 0 blocks
==22396==    indirectly lost: 0 bytes in 0 blocks
==22396==      possibly lost: 0 bytes in 0 blocks
==22396==    still reachable: 72,704 bytes in 1 blocks
==22396==         suppressed: 0 bytes in 0 blocks
==22396== Rerun with --leak-check=full to see details of leaked memory
==22396== 
==22396== For counts of detected and suppressed errors, rerun with: -v
==22396== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

Answer 1

First of all read the valgrind output VERY CAREFULLY and ENTIRELY (and do everything what valgrind recommends). In your case the fragments of interest are:

Rerun with --leak-check=full to see details of leaked memory

still reachable: 72,704 bytes in 1 blocks

You should also be aware of the following: http://valgrind.org/docs/manual/faq.html#faq.reports (4.1. My program uses the C++ STL and string classes. Valgrind reports 'still reachable' memory leaks involving these classes at the exit of the program, but there should be none).

And if you have time you may strongly benefit from detailed understanding of this: http://valgrind.org/docs/manual/mc-manual.html#mc-manual.leaks (4.2.8. Memory leak detection).

Finally you can find tons of useful info here: http://valgrind.org (valgrind homepage).

Answer 2

A strong grasp of basic data structures is good! I encourage you to explore it fully. Once you have grip on the basics then the STL can be very useful to take care of the busy work while you think about the hard problems.

There are no leaks in your code. Valgrind is seeing the memory elsewhere, as noted you can use the --leak-check=full to get more details. Its good that you are trying to understand those details.

This is a dangling pointer problem however.

#include <stdio.h>

template <class T>
class List {
    T* list;

public:
    int length;

    List(int len) {
        list = new T[len];
        length = len;
    }

    virtual ~List() {
        delete[] list;
    }

    T get(int index) {
        return list[index];
    }

    void set(int index, T val) {
        list[index] = val;
    }
};
/*
    You shouldn't change the code below, unless you want to _temporarily_ change the main function while testing.
    Change it back when you're done.
*/
typedef struct Point_ {
    int x;
    int y;
} Point;

int main(){
    List<int> integers(10);
    for(int i = 0; i < integers.length; i++){
        integers.set(i, i * 100);
        printf("%d ", integers.get(i));
    }
    printf("\n"); // this loop should print: 0 100 200 300 400 500 600 700 800 900

    List<Point *> points(5);
    for(int i = 0; i < points.length; i++) {
        Point *p = new Point;
        p->x = i * 10;
        p->y = i * 100;
        points.set(i, p);
        printf("(%d, %d) ", points.get(i)->x, points.get(i)->y);
        delete p;
    }
    Point* fail = points.get(0);
    fail->x = 0;
    fail->y = 0;
    printf("\n"); // this loop should print: (0, 0) (10, 100) (20, 200) (30, 300) (40, 400)
}

Note that I added the 'fail' variable after the points loop to demonstrate the issue. The points variable is declared and populated fine but then the contents are deleted. The List holds Point* rather than Point, as such when the delete occurs the pointer in the List is 'dangling'. Dangling pointer use will crash or corrupt the heap almost every time. This is a common problem with the STL containers as well when holding pointers.

A big flaw in C++ is the heap management. Correctly using the heap is very difficult with the biggest problem being ownership hand off. Dangling pointers or leaks often result from confusion on who owns the pointer. There is a brief article here Avoiding Leaks in C++

Running this program under Valgrind will show Invalid write errors. Give it a try.