LinuxQuestions.org

LinuxQuestions.org (/questions/)
-   Linux - Software (https://www.linuxquestions.org/questions/linux-software-2/)
-   -   Could Valgrind be wrong? (https://www.linuxquestions.org/questions/linux-software-2/could-valgrind-be-wrong-746089/)

ETCKerry 08-08-2009 03:54 PM

Could Valgrind be wrong?
 
Hello,

I'm trying to track down a memory leak using Valgrind, and was surprised by the number of "definitely lost" messages it threw at me, especially in one case where I have the following:

- A template class that manages a list of pointers (MY_LIST)
- A data object (DATA)
- A class that contains a MY_LIST and many pointers to DATA objects (CREATOR)

Some code snippets:

From my_list_class.h (some functions omitted):
Code:

template <class T>
class MY_LIST
{
public:
        // Constructor
        MY_LIST();

        // Destructor
        ~MY_LIST();

        // Private data accessors
        int Add(T *ToAdd);
        void Remove(const int &Index);
        inline int GetCount(void) const { return Count; };

        // Removes all objects from the list
        void Clear(void);

        // Operators
        T *operator [](const int &Index) const;

private:
        // The number of objects in this list
        int Count;

        // The data in the list
        T **ObjectList;
};

template <class T>
MY_LIST<T>::MY_LIST()
{
        // Initialize the count to zero and the pointers to NULL
        Count = 0;
        ObjectList = NULL;
}

template <class T>
MY_LIST<T>::~MY_LIST()
{
        // Remove all of the items in this list
        Clear();
}

template <class T>
void MY_LIST<T>::Clear(void)
{
        // Delete all of the items in the list
        int i;
        for (i = 0; i < Count; i++)
        {
                delete ObjectList[i];
                ObjectList[i] = NULL;
        }

        // Delete the list and set the count to zero
        delete [] ObjectList;
        ObjectList = NULL;
        Count = 0;

        return;
}

So when a MY_LIST is deleted, it deletes all of the objects that it's list of pointers point to, as well as the list itself.

The DATA object's constructor takes a pointer to a list, and it adds itself to the list in the constructor. So DATA::DATA() looks something like this:
Code:

DATA::DATA(MY_LIST<DATA>* List)
{
        List->Add(this);
}

And then I create a bunch of DATA objects in the CREATOR class, but I don't explicitly delete them. Instead, I call MY_LIST::Clear. This class looks like this:
Code:

// This class contains a private MY_LIST<DATA> object called List
CREATOR::CREATOR()
{
        // Make a bunch of DATA objects
        DATA* Data1 = new DATA(this);
        DATA* Data2 = new DATA(this);
        DATA* Data3 = new DATA(this);
        // and a whole lot more...
}

CREATOR::~CREATOR()
{
        List.Clear();
}

When I run Valgrind, it tells me every one of my DATA objects has leaked. I don't believe that this is correct, but I read that when Valgrind says "definitely lost" it is 100% sure.

I had CREATOR::CREATOR() print the addresses of the new objects to file, and I had MY_LIST::Clear() print the addresses of the object just before deletion to file, and the lists are identical... so this is a case where Valgrind is wrong?

Any ideas? I can post more code or a complete example if it will help.

Thanks,

Kerry

jeromeNP7 08-10-2009 08:27 AM

I would vote for Valgrind. C++ compilation is a complex task and a lot has to be done beyond what is implied by your own code. As a test you could compile your code with another C++ compiler and see if Valgrind throws different results.

Linux

zhjim 08-10-2009 08:45 AM

I'd also would give the point to valgrind, but who knows. I just noticed something in

my_list_class.h within the clear Function
Code:

for (i = 0; i < Count; i++)
        {
                delete ObjectList[i];
                ObjectList[i] = NULL;
        }

You just kill the ObjectList[i] and afterwards assign a varibale to it... Maybe that does not get interpreted very well.
Nother thing I read up is that if you just pass on a pointer value to a another pointer it can lead to memory leakage.
I just found to links that might be of some help.

http://www.yolinux.com/TUTORIALS/C++...moryLeaks.html
This one just talks about memory stuff within c and c++.

http://www.yolinux.com/TUTORIALS/C++...moryLeaks.html
This one just names other programms that look for memory leakage like valgrind

Regards Zhjim

johnsfine 08-10-2009 09:28 AM

Quote:

Originally Posted by zhjim (Post 3637751)
You just kill the ObjectList[i] and afterwards assign a varibale to it...

No. He deleted an object using a pointer, then he assigned NULL to the pointer (not to the object). That is perfectly valid. It is often the only correct simple design in situations very similar to that one. In this case, it was unnecessary, but it still is perfectly OK.

Quote:

Nother thing I read up is that if you just pass on a pointer value to a another pointer it can lead to memory leakage.
Discussion of general programming patterns that could be error prone is not constructive at this level. The author of the code is obviously not a beginner. The avoidance of memory leaks in the posted code seems to be carefully thought out.

I don't see the memory leak. I'm pretty good at spotting them, so while I can't guarantee I didn't miss something, I don't think any memory leak is indicated by the posted code. I have no estimate of whether that means Valgrind is wrong or it means the memory leak is a consequence of some code that wasn't posted.

zhjim 08-11-2009 05:36 AM

Quote:

Originally Posted by johnsfine
Discussion of general programming patterns that could be error prone is not constructive at this level. The author of the code is obviously not a beginner. The avoidance of memory leaks in the posted code seems to be carefully thought out.

Hard to guess the level a programmer has but your definetly right. I shut my gab and let the big ones take over ;)

ETCKerry 08-14-2009 12:37 PM

The point goes to Valgrind. It took me quite a bit of time to track it down, but I oversimplified in my original post and it turns out there really was a leak, but it was a result of code that wasn't posted. Just thought I'd check back to even the score...

Thanks for all the replies.

-Kerry


All times are GMT -5. The time now is 06:02 PM.