0

I am using c++ to implement a single link list of integers. My program simply asks the user to fill the list with 5 integers, the program should then delete any even integer and print the list after deletion.

Here is my code:

#include <iostream>
using namespace std;
class IntSLLNode
{
public:
    IntSLLNode()     { next = 0; }
    IntSLLNode(int i, IntSLLNode *ptr = 0)
    {
        info = i;
        next = ptr;
    }
    int info;
    IntSLLNode *next;
};
class IntSLList
{
public:
    IntSLList() {head = tail =0; }
    void AddToTail(int);
    void DeleteNode(int);
    void DisplayList();
    void deleteEven();
    IntSLLNode * getHead()
    {
        return head;
    }
private:
    IntSLLNode *head, *tail;
};
void IntSLList::AddToTail(int el)
{
    if (tail != 0) // if list not empty;
    {   tail->next = new IntSLLNode(el);
        tail = tail->next;
    }
    else
        head = tail = new IntSLLNode(el);
}

void IntSLList::deleteEven()
{
IntSLLNode *current;
current=head;
int  num;
while (current!=0)
{
    num=current->info;
    current=current->next;
    if(num%2==0)
    {
        DeleteNode(num);
    }
}
}
void IntSLList::DeleteNode(int el)
{
    if(head !=0)
        if(head==tail && el==head->info)
        {
            delete head;
            head=tail=0;
        }
        else if(el==head->info)
        {
            IntSLLNode *tmp=head;
            head=head->next;
            delete tmp;
        }
        else
        {
            IntSLLNode *pred, *tmp;
            for(pred=head, tmp=head->next;
                tmp!=0 && !(tmp->info==el);
                pred=pred->next, tmp=tmp->next);
            if(tmp!=0)
            {
                pred->next=tmp->next;
                if(tmp==tail)
                    tail=pred;
                delete tmp;
            }
        }
}

void IntSLList::DisplayList()
{
    IntSLLNode *current;
    current=head;
    if(current==0)
        cout<<"Empty List!";
    while (current!=0)
    {
        cout<<current->info<<" ";
        current=current->next;
    }
}

I got Unhandled exception at 0x002c1744 in ex4.exe: 0xC0000005: Access violation reading location 0xfeeefeee in statment int num=current->info; Can anyone suggest how to solve this?

user1477701
  • 147
  • 1
  • 5
  • 15
  • Perhaps your code would like to be properly formatted? – kviiri Nov 01 '13 at 21:51
  • Look at that error as a not-so-subtle hint that the address held in the pointer `current` being dereferenced is bogus. Now walk backward in your code and see if you can see how it could be so. Consider what your `DeleteNode` function does, and what it could possibly mean to the *caller* of that function, in this case `deleteEven()`. What do you think `current` is pointing to when `DeleteNode()` returns and you're back in `deleteEven()` ? – WhozCraig Nov 01 '13 at 21:55
  • Where is `num=current->info` line? – MahanGM Nov 01 '13 at 22:06
  • 0xfeeefeee means you are accessing a pointer that you freed: http://stackoverflow.com/questions/127386/in-visual-studio-c-what-are-the-memory-allocation-representations – drescherjm Nov 01 '13 at 22:13
  • seems unnecessary to look up the int again when you delete, why not just pass the pointer to DeleteNode? – AndersK Nov 02 '13 at 09:38

1 Answers1

1

I'm not sure about this, but I think the problem is laid here:

void IntSLList::deleteEven()
{
    IntSLLNode *current;
    current=head;
    while (current!=0)
    {
        if(current->info%2==0)
            DeleteNode(current->info);
        current=current->next; // this line
    }
}

I think there is a relation between deleted node which is preformed in the if statement and your next line. If you delete that specific pointer which points to an element in DeleteNode() then your current which it might be deleted till now will be pointing to a wrong address.

EDIT

void IntSLList::deleteEven()
{
    IntSLLNode *current;
    current=head;
    while (current!=0)
    {
        if(current->info%2==0)
        {
            int ind = current->info;

            current=current->next;
            DeleteNode(ind);
        }
        else
        {
          current=current->next;
        }
    }
}
MahanGM
  • 2,352
  • 5
  • 32
  • 45
  • Thanks a lot MahanGM :) I placed current=current->next; before the deletion and it works perfectly. I updated my code. – user1477701 Nov 01 '13 at 22:38
  • @user1477701 if that is all you did and it "works perfectly" consider it yet-more undefined behavior. Moving that line means you're now deleting the node *after* the one you wanted to delete, and you're *still* dereferencing an invalid node pointer on the next go around, only now *before* rather than after DeleteNode. This answer is correct, but your solution certainly is *not* if implemented as you described. – WhozCraig Nov 01 '13 at 23:50
  • @user1477701 WhozCraig is telling the right thing. You have to set your current node based on that `if` statement and yet I'm not sure again because your code has a bad implementation. Check my edit. – MahanGM Nov 02 '13 at 15:15