How to prevent memory leaks in the below code? I added "delete sentinel" before return statement. But how to deal with the memory leak of these 2 lines d->next = new ListNode(sum % 10)
and d->next = new ListNode(1)
ListNode* addTwoNumbers(ListNode* l1, ListNode* l2) {
ListNode* c1 = l1;
ListNode* c2 = l2;
ListNode* sentinel = new ListNode(0);
ListNode* d = sentinel;
int sum = 0;
while(c1 != NULL || c2 != NULL) {
sum /=10;
if(c1 != NULL){
sum += c1->val;
c1 = c1->next;
}
if(c2!= NULL){
sum += c2->val;
c2 = c2->next;
}
d->next = new ListNode(sum % 10);
d = d->next;
}
if(sum /10 == 1){
d->next = new ListNode(1);
}
return sentinel->next;
}
};
Answer
Your code is really confusing... Not because the algorithm is complex, but because you use several variables to point to the same thing. Why this multitude of unused variables ?
ListNode* addTwoNumbers(ListNode* l1, ListNode* l2)
{
ListNode* sentinel = new ListNode(0);
listNode* d = sentinel;
int sum = 0;
while (l1 != NULL || l2 != NULL) // use pointers l1 and l2, since passed by value.
{
if (l1 != NULL)
{
sum += l1->val;
l1 = l1->next;
}
if (l2 != NULL)
{
sum += l2->val;
l2 = l2->next;
}
d->next = new ListNode(sum % 10);
sum /= 10;
d = d->next;
}
if (sum)
{
d->next = new ListNode(sum);
}
// And this is where one of your memory leaks was...
if (sentinel->next)
{
d = sentinel->next; // delete useless sentinel.
sentinel->next = NULL; // important ! see ~ListNode()
delete sentinel;
sentinel = d;
}
return sentinel; // and not result->next.
}
To avoid leakage from a list, you need to write a proper list destructor to free child nodes and other dependent resources, if any. This is a rather recurrent theme in c++.
ListNode::~ListNode()
{
// never destroy lists recursively !! Destroying a large list
// that way could bust the stack, hence this odd looking loop.
for (ListNode * p = next, * q = NULL; p != NULL; p = q)
{
q = p->next;
p->next = NULL; // <-- avoids recursion.
delete p;
}
}
No comments:
Post a Comment