C++

Forum

 
Linked List program - Runtime Error

goonershael (1)
I normally work with the STL and love the features that it offers
But just now i wanted to test my skills with programming:

Can anybody help me with this code?

With MS c/c++ Compiler it is giving me runtime error!!!

#include <iostream>
#include <algorithm>
#include <cmath>

using namespace std;

class List{
protected:
struct node{
int info;
struct node *next;
};
typedef struct node *NODEPTR;
NODEPTR listptr;
public:

List(){
listptr=0;
}

~List(){
NODEPTR p,q;
if(emptyList())
;
for(p=listptr,q=p->next;p!=0;p=q,q=p->next)
delete p;
}


int emptyList(){
return (listptr==0);
}


void insertAfter(int oldvalue,int newvalue){
NODEPTR p,q;
for(p=listptr;p!=0 && p->info!=oldvalue;p=p->next)
;
if(p==0)
perror("ERROR: No value of this sought");
q = new node;
q->info = newvalue;
q->next = p->next;
q = listptr;
}

void push(int newvalue){
NODEPTR p;
p=new node;
p->info = newvalue;
p->next = listptr;
listptr = p;
}

void del(int oldvalue){
NODEPTR p;
NODEPTR q;
for(p=listptr,q=0;p!=0 && p-> info!=oldvalue ;q=p,p=p->next)
;
if(p==0)
perror("ERROR:no element in the list with thie value");
if(q==0)
listptr = p->next;
else
q->next = p->next;
delete p;
}

int pop(){
NODEPTR p;
int x;
if(emptyList())
perror("ERROR:List is empty");
p=listptr;
listptr = p;
x=p->info;
delete p;
return x;
}

void display(){
NODEPTR p;
for(p=listptr;p!=0;p->next){
cout << p->info;
}
}

};

//runner program :

int main(){
List l1;
for( int i=1;i<=5;i++){
l1.push(i*5);
}
l1.pop();
l1.pop();
l1.insertAfter(20,20);
l1.insertAfter(20,25);
l1.del(5);
l1.del(15);
l1.display();
return 0;
}


Please help me rectifying the mistake that I`m making here!!!
ne555 (10692)
1- Your code, indented
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
 
#include <algorithm>
#include <cmath>
#include <iostream>

using namespace std;

class List {
protected:
	struct node {
		int info;
		struct node *next;
	};
	typedef struct node *NODEPTR;
	NODEPTR listptr;

public:
	List() {
		listptr = 0;
	}

	~List() {
		NODEPTR p, q;
		if(emptyList())
			;
		for(p = listptr, q = p->next; p != 0; p = q, q = p->next)
			delete p;
	}

	int emptyList() {
		return (listptr == 0);
	}

	void insertAfter(int oldvalue, int newvalue) {
		NODEPTR p, q;
		for(p = listptr; p != 0 && p->info != oldvalue; p = p->next)
			;
		if(p == 0)
			perror("ERROR: No value of this sought");
		q = new node;
		q->info = newvalue;
		q->next = p->next;
		q = listptr;
	}

	void push(int newvalue) {
		NODEPTR p;
		p = new node;
		p->info = newvalue;
		p->next = listptr;
		listptr = p;
	}

	void del(int oldvalue) {
		NODEPTR p;
		NODEPTR q;
		for(p = listptr, q = 0; p != 0 && p->info != oldvalue;
		    q = p, p = p->next)
			;
		if(p == 0)
			perror("ERROR:no element in the list with thie value");
		if(q == 0)
			listptr = p->next;
		else
			q->next = p->next;
		delete p;
	}

	int pop() {
		NODEPTR p;
		int x;
		if(emptyList())
			perror("ERROR:List is empty");
		p = listptr;
		listptr = p;
		x = p->info;
		delete p;
		return x;
	}

	void display() {
		NODEPTR p;
		for(p = listptr; p != 0; p->next) {
			cout << p->info;
		}
	}
};

// runner program :

int main() {
	List l1;
	for(int i = 1; i <= 5; i++) {
		l1.push(i * 5);
	}
	l1.pop();
	l1.pop();
	l1.insertAfter(20, 20);
	l1.insertAfter(20, 25);
	l1.del(5);
	l1.del(15);
	l1.display();
	return 0;
}

a- Compilling with warnings 
foo.cpp|24 col 4| warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body]
foo.cpp|82 col 31| warning: for increment expression has no effect [-Wunused-value]
||    for(p = listptr; p != 0; p->next) {
||                             ~~~^~~~
so, an infinite loop in `display()'
you need to update p, p = p->next

\alpha- Running through a debugger 
(gdb) run
Program received signal SIGABRT, Aborted.
0x00007ffff71cc04f in raise () from /usr/lib/libc.so.6
(gdb) backtrace 
#0  0x00007ffff71cc04f in raise () from /usr/lib/libc.so.6
#1  0x00007ffff71cd47a in abort () from /usr/lib/libc.so.6
#2  0x00007ffff7209c50 in __libc_message () from /usr/lib/libc.so.6
#3  0x00007ffff720ffe6 in malloc_printerr () from /usr/lib/libc.so.6
#4  0x00007ffff72107de in _int_free () from /usr/lib/libc.so.6
#5  0x0000000000400c0f in List::pop (this=0x7fffffffdfa0) at foo.cpp:76
#6  0x00000000004008de in main () at foo.cpp:96
(gdb) frame 5
#5  0x0000000000400c0f in List::pop (this=0x7fffffffdfa0) at foo.cpp:76
76			delete p;
(gdb) print p
$1 = (List::node *) 0x614ca0
(gdb) list
71			if(emptyList())
72				perror("ERROR:List is empty");
73			p = listptr;
74			listptr = p;
75			x = p->info;
76			delete p;
77			return x;
78		}
You never update the head of the list after doing pop(), so you are trying to delete an invalid pointer.
dhayden (5795)
Please use code tags. Highlight your code and click the <> button to the right of the edit window. This makes your code look like this: (more comments below)
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
 
#include <iostream>
#include <algorithm>
#include <cmath>
#include <cstdio>

using namespace std;

class List
{
protected:
    struct node
    {
	int info;
	struct node *next;
    };
    typedef struct node *NODEPTR;
    NODEPTR listptr;
public:

    List()
    {
	listptr = 0;
    }

    ~List()
    {
	NODEPTR p, q;
	if (emptyList());
	for (p = listptr, q = p->next; p != 0; p = q, q = p->next)
	    delete p;
    }

    int emptyList()
    {
	return (listptr == 0);
    }

    void insertAfter(int oldvalue, int newvalue)
    {
	NODEPTR p, q;
	for (p = listptr; p != 0 && p->info != oldvalue; p = p->next);
	if (p == 0)
	    perror("ERROR: No value of this sought");
	q = new node;
	q->info = newvalue;
	q->next = p->next;
	q = listptr;
    }

    void push(int newvalue)
    {
	NODEPTR p;
	p = new node;
	p->info = newvalue;
	p->next = listptr;
	listptr = p;
    }

    void del(int oldvalue)
    {
	NODEPTR p;
	NODEPTR q;
	for (p = listptr, q = 0; p != 0 && p->info != oldvalue; q = p, p = p->next);
	if (p == 0)
	    perror("ERROR:no element in the list with thie value");
	if (q == 0)
	    listptr = p->next;
	else
	    q->next = p->next;
	delete p;
    }

    int pop()
    {
	NODEPTR p;
	int x;
	if (emptyList())
	    perror("ERROR:List is empty");
	p = listptr;
	listptr = p;
	x = p->info;
	delete p;
	return x;
    }

    void display()
    {
	NODEPTR p;
	for (p = listptr; p != 0; p->next) {
	    cout << p->info;
	}
    }

};

//runner program :

int
main()
{
    List l1;
    for (int i = 1; i <= 5; i++) {
	l1.push(i * 5);
    }
    l1.pop();
    l1.pop();
    l1.insertAfter(20, 20);
    l1.insertAfter(20, 25);
    l1.del(5);
    l1.del(15);
    l1.display();
    return 0;
}

Line 28: This code does nothing.
Line 29: When you get the end of the list, p=q will set p to NULL and q = p->next will dereference a null pointer. The destructor could be written much easier as:
1
2
3
4
5
 
        while (listptr) {
            NODEPTR tmp = listptr->next;
            delete listptr;
            listptr = tmp;
        }


Line 43: If the value isn't found, the rest of the code still executes, and since p == NULL in that case, line 46 is accessing an NULL pointer.
Line 43: perror() prints the value of errno, which is inappropriate here. It would be better to print the message to the cerr stream:
cerr << "Error: No value of this sought\n";
LIne 47: You still need to link q into the list, so this should be p->next = q;
LIne 65: Same as above. If p == NULL then you still execute the code below it.
Line 80: Sound be listptr = p->next;
Line 89: The last part of the for loop should be p = p->next
Topic archived. No new replies allowed.