Skip to content

Commit 65efd47

Browse files
vil02Panquesito7
andauthored
fix: remove memory leak in reverse_a_linked_list.cpp (#2439)
* style: remove unused header * fix: add destructor to list * style: add missing constructors, make some methods const * fix: google-readability-braces-around-statements * fix: suppress some warnings * docs: remove meaningless documentation * docs: add missing documentation * style: check if isEmpty in copy_all_nodes_from_list * style: declare variables in seperate lines Co-authored-by: David Leal <[email protected]>
1 parent 1d2c5d9 commit 65efd47

File tree

1 file changed

+127
-25
lines changed

1 file changed

+127
-25
lines changed

data_structures/reverse_a_linked_list.cpp

Lines changed: 127 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424

2525
#include <cassert> /// for assert
2626
#include <iostream> /// for I/O operations
27-
#include <memory> /// for dynamic memory
2827
#include <new> /// for managing dynamic storage
2928

3029
/**
@@ -43,46 +42,65 @@ namespace linked_list {
4342
class Node {
4443
public:
4544
int32_t val; /// value of the current link
46-
Node *next; /// pointer to the next value on the list
45+
Node* next; /// pointer to the next value on the list
4746
};
4847

48+
/**
49+
* @brief creates a deep copy of a list starting at the input node
50+
* @param[in] node pointer to the first node/head of the list to be copied
51+
* @return pointer to the first node/head of the copied list or nullptr
52+
*/
53+
Node* copy_all_nodes(const Node* const node) {
54+
if (node) {
55+
// NOLINTNEXTLINE(cppcoreguidelines-owning-memory)
56+
Node* res = new Node();
57+
res->val = node->val;
58+
res->next = copy_all_nodes(node->next);
59+
return res;
60+
}
61+
return nullptr;
62+
}
63+
4964
/**
5065
* A list class containing a sequence of links
5166
*/
67+
// NOLINTNEXTLINE(cppcoreguidelines-special-member-functions)
5268
class list {
5369
private:
54-
Node *head; // link before the actual first element
70+
Node* head = nullptr; // link before the actual first element
71+
void delete_all_nodes();
72+
void copy_all_nodes_from_list(const list& other);
73+
5574
public:
56-
/**
57-
* List constructor. Initializes the first link.
58-
*/
59-
list() {
60-
head = nullptr; // Initialize the first link
61-
}
62-
bool isEmpty();
75+
bool isEmpty() const;
6376
void insert(int32_t new_elem);
6477
void reverseList();
65-
void display();
66-
int32_t top();
67-
int32_t last();
68-
int32_t traverse(int32_t index);
78+
void display() const;
79+
int32_t top() const;
80+
int32_t last() const;
81+
int32_t traverse(int32_t index) const;
82+
~list();
83+
list() = default;
84+
list(const list& other);
85+
list& operator=(const list& other);
6986
};
7087

7188
/**
7289
* @brief Utility function that checks if the list is empty
7390
* @returns true if the list is empty
7491
* @returns false if the list is not empty
7592
*/
76-
bool list::isEmpty() { return head == nullptr; }
93+
bool list::isEmpty() const { return head == nullptr; }
7794

7895
/**
7996
* @brief Utility function that adds a new element at the end of the list
8097
* @param new_elem element be added at the end of the list
8198
*/
8299
void list::insert(int32_t n) {
83100
try {
84-
Node *new_node = new Node();
85-
Node *temp = nullptr;
101+
// NOLINTNEXTLINE(cppcoreguidelines-owning-memory)
102+
Node* new_node = new Node();
103+
Node* temp = nullptr;
86104
new_node->val = n;
87105
new_node->next = nullptr;
88106
if (isEmpty()) {
@@ -94,7 +112,7 @@ void list::insert(int32_t n) {
94112
}
95113
temp->next = new_node;
96114
}
97-
} catch (std::bad_alloc &exception) {
115+
} catch (std::bad_alloc& exception) {
98116
std::cerr << "bad_alloc detected: " << exception.what() << "\n";
99117
}
100118
}
@@ -105,8 +123,9 @@ void list::insert(int32_t n) {
105123
* @returns void
106124
*/
107125
void list::reverseList() {
108-
Node *curr = head;
109-
Node *prev = nullptr, *next_node = nullptr;
126+
Node* curr = head;
127+
Node* prev = nullptr;
128+
Node* next_node = nullptr;
110129
while (curr != nullptr) {
111130
next_node = curr->next;
112131
curr->next = prev;
@@ -120,7 +139,7 @@ void list::reverseList() {
120139
* @brief Utility function to find the top element of the list
121140
* @returns the top element of the list
122141
*/
123-
int32_t list::top() {
142+
int32_t list::top() const {
124143
if (!isEmpty()) {
125144
return head->val;
126145
} else {
@@ -131,9 +150,9 @@ int32_t list::top() {
131150
* @brief Utility function to find the last element of the list
132151
* @returns the last element of the list
133152
*/
134-
int32_t list::last() {
153+
int32_t list::last() const {
135154
if (!isEmpty()) {
136-
Node *t = head;
155+
Node* t = head;
137156
while (t->next != nullptr) {
138157
t = t->next;
139158
}
@@ -146,8 +165,8 @@ int32_t list::last() {
146165
* @brief Utility function to find the i th element of the list
147166
* @returns the i th element of the list
148167
*/
149-
int32_t list::traverse(int32_t index) {
150-
Node *current = head;
168+
int32_t list::traverse(int32_t index) const {
169+
Node* current = head;
151170

152171
int count = 0;
153172
while (current != nullptr) {
@@ -163,6 +182,42 @@ int32_t list::traverse(int32_t index) {
163182
exit(1);
164183
}
165184

185+
/**
186+
* @brief calls delete operator on every node in the represented list
187+
*/
188+
void list::delete_all_nodes() {
189+
while (head != nullptr) {
190+
const auto tmp_node = head->next;
191+
delete head;
192+
head = tmp_node;
193+
}
194+
}
195+
196+
list::~list() { delete_all_nodes(); }
197+
198+
void list::copy_all_nodes_from_list(const list& other) {
199+
assert(isEmpty());
200+
head = copy_all_nodes(other.head);
201+
}
202+
203+
/**
204+
* @brief copy constructor creating a deep copy of every node of the input
205+
*/
206+
list::list(const list& other) { copy_all_nodes_from_list(other); }
207+
208+
/**
209+
* @brief assignment operator creating a deep copy of every node of the input
210+
*/
211+
list& list::operator=(const list& other) {
212+
if (this == &other) {
213+
return *this;
214+
}
215+
delete_all_nodes();
216+
217+
copy_all_nodes_from_list(other);
218+
return *this;
219+
}
220+
166221
} // namespace linked_list
167222
} // namespace data_structures
168223

@@ -194,11 +249,58 @@ static void test() {
194249
std::cout << "All tests have successfully passed!" << std::endl;
195250
}
196251

252+
void test_copy_constructor() {
253+
data_structures::linked_list::list L;
254+
L.insert(10);
255+
L.insert(20);
256+
L.insert(30);
257+
data_structures::linked_list::list otherList(L);
258+
otherList.insert(40);
259+
260+
L.insert(400);
261+
262+
assert(L.top() == 10);
263+
assert(otherList.top() == 10);
264+
assert(L.traverse(1) == 20);
265+
assert(otherList.traverse(1) == 20);
266+
267+
assert(L.traverse(2) == 30);
268+
assert(otherList.traverse(2) == 30);
269+
270+
assert(L.last() == 400);
271+
assert(otherList.last() == 40);
272+
}
273+
274+
void test_assignment_operator() {
275+
data_structures::linked_list::list L;
276+
data_structures::linked_list::list otherList;
277+
L.insert(10);
278+
L.insert(20);
279+
L.insert(30);
280+
otherList = L;
281+
282+
otherList.insert(40);
283+
L.insert(400);
284+
285+
assert(L.top() == 10);
286+
assert(otherList.top() == 10);
287+
assert(L.traverse(1) == 20);
288+
assert(otherList.traverse(1) == 20);
289+
290+
assert(L.traverse(2) == 30);
291+
assert(otherList.traverse(2) == 30);
292+
293+
assert(L.last() == 400);
294+
assert(otherList.last() == 40);
295+
}
296+
197297
/**
198298
* @brief Main function
199299
* @returns 0 on exit
200300
*/
201301
int main() {
202302
test(); // run self-test implementations
303+
test_copy_constructor();
304+
test_assignment_operator();
203305
return 0;
204306
}

0 commit comments

Comments
 (0)