C++

Forum

 
Valgrind - memory leaks

Jaimito (14)
I´m trying to optimize my linkdelist´s memory management, but I got stuck.
I have a class sortedlist which consist of class student and struct listnode. I got stuck with writing propper destructor, copy constuctor and copy asignment operator. I wrote a few test cases, but valgrind returns some errors and some memory leaks, which I care about the most:

==16594== Memcheck, a memory error detector
==16594== Command: runMain --leak-check=fullmak
==16594==
==16594== Conditional jump or move depends on uninitialised value(s)
==16594== at 0x400EF7: SortedList::insert(Student*) (SortedList.cpp:83)
==16594==
==16594== Conditional jump or move depends on uninitialised value(s)
==16594== at 0x400E6F: SortedList::copyList(SortedList::Listnode*) (SortedList.cpp:44)
==16594== by 0x400D44: SortedList::SortedList(SortedList const&) (SortedList.cpp:13)
==16594==
==16594== Use of uninitialised value of size 8
==16594== at 0x400E1E: SortedList::copyList(SortedList::Listnode*) (SortedList.cpp:48)
==16594== by 0x400D44: SortedList::SortedList(SortedList const&) (SortedList.cpp:13)
==16594==
==16594== Use of uninitialised value of size 8
==16594== at 0x400E5D: SortedList::copyList(SortedList::Listnode*) (SortedList.cpp:59)
==16594== by 0x400D44: SortedList::SortedList(SortedList const&) (SortedList.cpp:13)
==16594==
==16594== Conditional jump or move depends on uninitialised value(s)
==16594== at 0x400E6F: SortedList::copyList(SortedList::Listnode*) (SortedList.cpp:44)
==16594== by 0x400D44: SortedList::SortedList(SortedList const&) (SortedList.cpp:13)
==16594==
==16594== Use of uninitialised value of size 8
==16594== at 0x400E1E: SortedList::copyList(SortedList::Listnode*) (SortedList.cpp:48)
==16594== by 0x400D44: SortedList::SortedList(SortedList const&) (SortedList.cpp:13)
==16594==
==16594== Use of uninitialised value of size 8
==16594== at 0x400E5D: SortedList::copyList(SortedList::Listnode*) (SortedList.cpp:59)
==16594== by 0x400D44: SortedList::SortedList(SortedList const&) (SortedList.cpp:13)
==16594==
==16594== Invalid read of size 8
==16594== at 0x400E1E: SortedList::copyList(SortedList::Listnode*) (SortedList.cpp:48)
==16594== by 0x400D44: SortedList::SortedList(SortedList const&) (SortedList.cpp:13)
==16594== Address 0x29aee225ffffff is not stack'd, malloc'd or (recently) free'd
==16594==
==16594==
==16594== Process terminating with default action of signal 11 (SIGSEGV)
==16594== General Protection Fault
==16594== at 0x400E1E: SortedList::copyList(SortedList::Listnode*) (SortedList.cpp:48)
==16594== by 0x400D44: SortedList::SortedList(SortedList const&) (SortedList.cpp:13)
==16594==
==16594== HEAP SUMMARY:
==16594== in use at exit: 112 bytes in 7 blocks
==16594== total heap usage: 7 allocs, 0 frees, 112 bytes allocated
==16594==
==16594== LEAK SUMMARY:
==16594== definitely lost: 32 bytes in 2 blocks
==16594== indirectly lost: 16 bytes in 1 blocks
==16594== possibly lost: 0 bytes in 0 blocks
==16594== still reachable: 64 bytes in 4 blocks
==16594== suppressed: 0 bytes in 0 blocks
==16594== Rerun with --leak-check=full to see details of leaked memory
==16594==
==16594== For counts of detected and suppressed errors, rerun with: -v
==16594== Use --track-origins=yes to see where uninitialised values come from
==16594== ERROR SUMMARY: 8 errors from 8 contexts (suppressed: 6 from 6)
Segmentation fault (core dumped)


It gives me 4 commands, which causes errors in SortedList.cpp:
while(ptr != NULL) in copyList at 44
newListnode->student = ptr->student; in copyList at 48
ptr = ptr->next; in copyList at 59
if(Ptr != NULL) in insert at 83

Can you give me any advices how to solve these errors and prevent the memory leaks? I'd be very grateful. Here is my code:

Student.h
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
 
#ifndef STUDENT_H
#define STUDENT_H

class Student {

  public:
    
    Student();
    Student(int ID);
    Student(int ID, int cr, double grPtAv);
    Student(const Student &);
    Student & operator=(const Student &);
    ~Student();

    int getID() const;
    int getCredits() const;
    double getGPA() const;
    void update(char grade, int cr);
    void print() const;

  private:
    int studentID;
    int credits;
    double GPA;
};
#endif


Student.cpp - omited

SortedList.h
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
 
#ifndef SORTEDLIST_H
#define SORTEDLIST_H

#include "Student.h"

class SortedList {

  public:
    
    SortedList();
    SortedList(const SortedList &);
    SortedList & operator=(const SortedList &);
    ~SortedList();

    bool insert(Student *s);
    Student *find(int studentID);
    Student *remove(int studentID);
    void print() const;

  private:
    struct Listnode {    
      Student *student;
      Listnode *next;
    };
    Listnode *head;
    static void      freeList (Listnode *L);
    static Listnode *copyList (Listnode *L);
};

#endif


SortedList.cpp
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
 
#include <iostream>
#include "SortedList.h"
#include "Student.h"

using namespace std;

SortedList::SortedList() {
	head == NULL;
}

SortedList::SortedList(const SortedList & list) {
	head == NULL;
	head = list.copyList(head);
}

SortedList & SortedList::operator=(const SortedList & list) {
	if(this != &list) {
		list.freeList(head);
		head == NULL;
		head = list.copyList(head);
		}
	return *this;
}

SortedList::~SortedList() {
	this->freeList(head);
}

void SortedList::freeList(SortedList::Listnode *L) {
	SortedList::Listnode *tmp;
	while(L != NULL) {
		tmp = L->next;
		delete L;
		L = tmp;
	}
}

SortedList::Listnode *SortedList::copyList (SortedList::Listnode *L) {
	SortedList::Listnode *ptr = L;
	SortedList::Listnode *tmp = NULL;
	while(ptr != NULL) {
		SortedList::Listnode *newListnode = new Listnode;
		newListnode->student = ptr->student;
		newListnode->next = NULL;

	    if (tmp != NULL) {
	    	tmp->next = newListnode;
	     } else {
	    	 L = newListnode;
	     }

	    tmp = newListnode;
	    ptr = ptr->next;
	}
	return L;
}

bool SortedList::insert(Student *s) {
	Listnode *newListnode = new Listnode();
	newListnode->student = s;
	newListnode->next = NULL;

	Student *stdnt = newListnode->student;
	int new_ide = (*stdnt).getID();
	
	Listnode *Ptr = head;
	Listnode *Ptr_prv = head;
	
	if(Ptr != NULL) {
		int ide = Ptr->student->getID();
		int head_ide = head->student->getID();
		
		if(ide == new_ide) {
			return false;
		}
		if(new_ide < head_ide) {
			head = newListnode;
			head->next = Ptr;
			return true;
		}
		
		while( Ptr ) {
			ide = Ptr->student->getID();
			if(ide == new_ide) {
				return false;
			}
			if(new_ide < ide) {
				Ptr_prv->next = newListnode;
				newListnode->next = Ptr;
				return true;
	     	}
			if(Ptr->next == NULL) {
				Ptr->next = newListnode;
				return true;
			} else {
				Ptr_prv = Ptr;
				Ptr = Ptr->next;
			}
		}
	} else {
    	head = newListnode;
    	return true;
	}
}

Student *SortedList::find(int studentID) {
	Listnode *Ptr = head;

	while( Ptr != NULL ) {
			int ide = Ptr->student->getID();
			if(ide == studentID) {
				return Ptr->student;
				}
		  	Ptr = Ptr->next;;
		    }
	return NULL;
}

Student *SortedList::remove(int studentID) {
	Listnode *Ptr = head;
	Listnode *Ptr_prv = head;
	
	if(Ptr == NULL) {
		head == NULL;
		return NULL;
	}
	
	int ide_head = head->student->getID();
	if(ide_head == studentID) {
		head = head->next;
		return Ptr->student;
	}
	
	if(Ptr->next == NULL) {
		Ptr->next == NULL;
		return Ptr->student;
	} else {
		 while( Ptr != NULL ) {
			int ide = Ptr->student->getID();

			if(ide == studentID) {
				Ptr_prv->next = Ptr->next;
				return Ptr->student;
				}
			
			Ptr_prv = Ptr;
			Ptr = Ptr->next;
	    }
	}
return NULL;
}

void SortedList::print() const {
	Listnode *Ptr = head;
	
	while( Ptr != NULL ) {
		Student* stdnt = Ptr->student;
		(*stdnt).print();
		cout << endl;
		Ptr = Ptr->next;
		}
}


main.cpp - test file
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
 
#include <iostream>
#include "SortedList.h"
#include "Student.h"

using namespace std;

int main() {


	SortedList sortedlist;

	Student student_01;
	Student student_02(222222);
	Student student_03(333333, 20, 4);

	Student student_04 = student_02;

	student_01 = student_04;

	Student student_05(student_04);

	sortedlist.insert(&student_01);
	sortedlist.insert(&student_02);
	sortedlist.insert(&student_03);

	student_02.update('A', 20);

	sortedlist.remove(222222);
	sortedlist.remove(333333);

	SortedList sortedlist2 = sortedlist;

	SortedList sortedlist3(sortedlist);

	sortedlist2.insert(&student_01);

	sortedlist3.insert(&student_03);

	return 0;
}
coder777 (8444)
On line 8 and following you keep writing head == NULL; which is a comparison not an assignment.
head remains uninitialized

[EDIT] The problem with == occurs more often (like on line 135)
Last edited on
writetonsharma (1461)
I think you are missing some switches which let valgrind tell where/line the memory was allocated and not freed. Check your run of valgrind and it will give you more accurate information about memory allocated and not freed.

You should try to free "definitely lost" memory first. Valgrind and other memory profiling tools like purify sometimes are not correct on other type of leaks. For example, in some cases memory allocated in one module and deallocated in other module, valgrid thinks its a memory leak.
Jaimito (14)
You were right, with the head == NULL; statement. It was a silly mistake. I fixed it, but still have to think up how to fix the memory leaks.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
 
==9319== Memcheck, a memory error detector
==9319== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
==9319== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info
==9319== Command: runMain --leak-check=full
==9319== 
==9319== 
==9319== HEAP SUMMARY:
==9319==     in use at exit: 32 bytes in 2 blocks
==9319==   total heap usage: 5 allocs, 3 frees, 80 bytes allocated
==9319== 
==9319== LEAK SUMMARY:
==9319==    definitely lost: 32 bytes in 2 blocks
==9319==    indirectly lost: 0 bytes in 0 blocks
==9319==      possibly lost: 0 bytes in 0 blocks
==9319==    still reachable: 0 bytes in 0 blocks
==9319==         suppressed: 0 bytes in 0 blocks
==9319== Rerun with --leak-check=full to see details of leaked memory
==9319== 
==9319== For counts of detected and suppressed errors, rerun with: -v
==9319== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 6 from 6)


I tracked down these 3 expressions in main.cpp, which leads to this leaks:
1
2
3
 
student_01 = student_04; // Copy asignment operator
sortedlist.remove(222222); // The student is removed from the list
sortedlist.remove(333333); // The student is removed from the list


Jaimito (14)
I made a adjustment and I hope it will work, but I need an advice regarding static function call. As you can see above I have two static functions declared in SortedList.h, which I want to use with Big 3. I just have a problem, cause these functions are supposed to call public functions of SortedList (insert and remove), which require object of SortedList.

The code below gives me these errors:

1
2
3
4
 
SortedList.cpp: In static member function âstatic void SortedList::freeList(SortedList::Listnode*)â:
SortedList.cpp:38: error: âthisâ is unavailable for static member functions
SortedList.cpp: In static member function âstatic SortedList::Listnode* SortedList::copyList(SortedList::Listnode*)â:
SortedList.cpp:62: error: âthisâ is unavailable for static member functions



1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
 
SortedList::SortedList(const SortedList & list) { // Copy constructor
	head = list.copyList(head);
}

SortedList & SortedList::operator=(const SortedList & list) { // Copy asignment operator
	if(this != &list) {
		list.freeList(head);
		head = list.copyList(head);
		}
	return *this;
}

SortedList::~SortedList() { // Destructor
	this->freeList(head);
}


void SortedList::freeList(SortedList::Listnode *L) { // Traverses the linked list, returning each node to free storage
	while(L != NULL) {
		this.remove(L->student);
	}
}


SortedList::Listnode *SortedList::copyList (SortedList::Listnode *L) { // Returns a pointer to the first node of a copy of original linked list
	// Loop over the elements of the passed list if any
	SortedList::Listnode *ptr = L;
	while(ptr != NULL) {
		this.insert(ptr->student);
		ptr = ptr->next; // Shift the loop variable along the passed list
	}
	return ptr;
}
writetonsharma (1461)
static functions are like global function and don't have a "this" pointer.

In case you want to call the class functions, either you need to pass the class object to these functions and then use that class object to call other functions of the class.

But, I want to understand why would you want static functions ? Why don't you let the interface to these functions from a class object ?
Jaimito (14)
So I almast solved my problem. I had to change a few functions in SortedLists.cpp... hopefully I'll finish it soon.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
 
SortedList::SortedList(const SortedList & list) { // Copy constructor
	head = NULL; // Init the head of the list
	head = copyList(list.head);
}

SortedList & SortedList::operator=(const SortedList & list) { // Copy asignment operator
	if(this != &list) {
		freeList(list.head);
		head = NULL; // Init the head of the list
		head = copyList(list.head);
		}
	return *this;
}

SortedList::~SortedList() { // Destructor
	freeList(head);
}


void SortedList::freeList(SortedList::Listnode *L) { // Traverses the linked list, returning each node to free storage
	SortedList::Listnode *ptr = L;
	SortedList::Listnode *tmp = NULL;
	while(ptr != NULL) {
		tmp = ptr->next;
		delete ptr->student;
		delete ptr;
		ptr = tmp;
	}
}


SortedList::Listnode *SortedList::copyList (SortedList::Listnode *L) { // Returns a pointer to the first node of a copy of original linked list
	// Loop over the elements of the passed list if any
	SortedList::Listnode *ptr = L;
	SortedList::Listnode *ptr_rtrn = NULL;
	SortedList::Listnode *tmp = NULL;
	while(ptr != NULL) {

		// Allocate a new node
		SortedList::Listnode *newListnode = new Listnode();

		// Allocate a new student and set the fields there
		Student::Student *newStudent = new Student(*(ptr->student));
		newListnode->student = newStudent;
		newListnode->next = NULL;

		 // Add new node to the local list
	    if (tmp != NULL) {
	    	tmp->next = newListnode;
	     } else {
	    	 ptr_rtrn = newListnode;
	     }

	    tmp = newListnode;
	    ptr = ptr->next; // Shift the loop variable along the passed list
	}
	return ptr_rtrn;
}


and also a bit my main file.
Topic archived. No new replies allowed.