what is wrong in the following linked list code

[ep.hobbyiest]

hi,
i am trying this linked list code from two days. but i don't know where i m wrong...
plz tell me where i m wrong..

#include<stdio.h>
#include<stdlib.h>
#include<malloc.h>


typedef struct Node
{
	int data;
	struct Node *next;
}node;


int main(void)
{
	int i;
	node *head,*tempp,*new_node;
	
	head =(node*)malloc(sizeof(node));
	head->data = 0;
	head->next = NULL;
	tempp = new_node = head;
	head = head->next;
	
	printf("\nstart\n");
	for(i=1;i<5;i++)
	{
		head = (node*)malloc(sizeof(node));
		if(head	==NULL)
		printf("\nNo memory is allocated\n");
		else
		{
			printf("\nMemory is allocated %d\n",i);
			head->data = i;
			head = head->next;
		}
	}
	printf("\nDone\n");	
	head = NULL;
	while(new_node!=NULL)
	{
		printf("Data : %d\n",new_node->data);
		new_node = new_node->next;		
	}
	printf("\nEnd of List\n");
	return 0;
}

 

[xenos]

You dont link nodes.
you do not update previous node's next pointer. Why the malloc outdide the loop?

 

[xenos]

Due to time pressure, did't loot after the first loop.
Some more errors:
you do not access the list at all while trying to print contents (head usage?)
you do not free the allocated memory at end (of course C RTL will do it uppon exit).

 

[bigdogguru]

Besides the important points mentioned by xenos, your code would certainly benefit by splitting the existing code and encapsulating its associated tasks into specific routines for each task. Use the "divide and conquer" methodology, one key benefit from this approach is the concept of reusable code, you can effectively reuse the existing routines within the design of another linked list, by simply change the definition of the NODE struct and a few modifications of the insert_node() routine. Or you could write a customized create_node() routine which the pointer to the newly created node can then be passed to the insert_node() routine. The possibilities are almost endless.

Instead of the lumping all the tasks into main() as it exists now:

---

Code C - [expand]
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
#include<stdio.h>
#include<stdlib.h>
#include<malloc.h>
 
 
typedef struct Node
{
    int data;
    struct Node *next;
}node;
 
 
int main(void)
{
    int i;
    node *head,*tempp,*new_node;
    
    head =(node*)malloc(sizeof(node));
    head->data = 0;
    head->next = NULL;
    tempp = new_node = head;
    head = head->next;
    
    printf("\nstart\n");
    for(i=1;i<5;i++)
    {
        head = (node*)malloc(sizeof(node));
        if(head ==NULL)
        printf("\nNo memory is allocated\n");
        else
        {
            printf("\nMemory is allocated %d\n",i);
            head->data = i;
            head = head->next;
        }
    }
    printf("\nDone\n"); 
    head = NULL;
    while(new_node!=NULL)
    {
        printf("Data : %d\n",new_node->data);
        new_node = new_node->next;      
    }
    printf("\nEnd of List\n");
    return 0;
}

---

Create the specific routines to then call within main(), the following example illustrates the concept including the use of pointer to a pointer variable parameter passing:

---

Code C - [expand]
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
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
// LinkedList.c : Defines the entry point for the console application.
//
 
#include<stdio.h>
#include<stdlib.h>
#include<malloc.h>
 
#define NEW_NODE 0
#define NO_MEM_ALLOC 1
#define FOUND 0
#define NOT_FOUND 2
#define TAIL_REACHED 3
 
 
 
typedef struct node
{
    int id;
    struct NODE *next;
}NODE;
 
NODE * init_list(void);
int insert_node(NODE * list, int id);
NODE * traverse_list(NODE * current);
int delete_node(NODE * list, int * id);
NODE * find_node(NODE * list, int id);
 
 
int main(void)
{
    int i;
    NODE * head = NULL;
    NODE * current = NULL;
 
    int search_value = 3;
 
 
    printf("\nStart List\n");
 
    printf("\nInsert NODEs\n");
 
    for (i = 1; i<5; i++)
    {
        insert_node(&head, i);
    }
 
    printf("\nInsertion Done\n");
 
    printf("\nSearch for NODE\n");
 
 
    if ((current = find_node(head, search_value)) == NULL)
    {
        printf("\nNODE not found\n");
    }
    else
    {
        printf("\nNODE found\n");
        printf("\nNODE Address: %lu, id: %d\n", current, current->id);
    }
 
    printf("\nSearch Done\n");
 
    printf("\nTraversing Linked List\n");
 
    current = head;
 
    while (current != NULL)
    {
        printf("\nid : %d\n", current->id);
 
        current = traverse_list(current);
    }
 
    printf("\nTraverse Done\n");
 
    printf("\nDeleting NODEs\n");
 
    current = NULL;
 
 
    for (i = 1; i < 5; i++)
    {
        current = find_node(head, i);
        delete_node(&head, current);
 
        printf("\nDeleted NODE with id: %d\n", i);
 
    }
 
    if (head == NULL)
    {
        printf("\nVariable head set to NULL\n");
    }
    else
    {
        printf("\nVariable head set to: %lu\n", head);
    }
 
    printf("\nDeletion Done\n");
 
    printf("\nEnd of List\n");
 
 
    return 0;
}
 
int insert_node(NODE ** list, int id)
{
    NODE * new_node;
 
    if ((new_node = (NODE *)malloc(sizeof(NODE))) == NULL)
    {
        printf("\nMemory for NODE is not allocated\n");
        exit(NO_MEM_ALLOC);
    }
    else
    {
        printf("\nMemory for NODE is allocated\n");
    }
 
    if (*list == NULL)
    {
        
        new_node->id = id;
        new_node->next = NULL;
        * list = new_node;
    }
    else
    {
        new_node->id = id;
        new_node->next = * list;
        * list = new_node;
    }
 
    return NEW_NODE;
}
 
NODE * traverse_list(NODE * current)
{
    NODE * previous;
 
    previous = current;
 
    current = previous->next;
 
    return current;
    
}
 
 
 
int delete_node(NODE ** list, NODE * target)
{
    NODE * current = *list;
 
    if (current == target)
    {
        current = target->next;
    
        *list = current;
        
        free(target);
        
        return(FOUND);
 
    }
 
    while (current->next != target)
    {
        if (current->next == NULL)
        {
            printf("\nTarget for deletion not found in linked list\n");
 
            exit(NOT_FOUND);
        }
 
        current = current->next;
    }
 
    
    current->next = target->next;
 
    free(target);
 
    return(FOUND);
}
 
NODE * find_node(NODE * list, int id)
{
    while (list->id != id)
    {
        if (list->next != NULL)
        {
            list = list->next;
        }
        else
        {
            return NULL;
        }
    }
 
    return list;
}

---

Output from above example:

Start List

Insert NODEs

Memory for NODE is allocated

Memory for NODE is allocated

Memory for NODE is allocated

Memory for NODE is allocated

Insertion Done

Search for NODE

NODE found

NODE Address: 5392608, id: 3

Search Done

Traversing Linked List

id : 4

id : 3

id : 2

id : 1

Traverse Done

Deleting NODEs

Deleted NODE with id: 1

Deleted NODE with id: 2

Deleted NODE with id: 3

Deleted NODE with id: 4

Variable head set to NULL

Deletion Done

End of List

I'm sure you have quite a few questions, so please review the above example and post any questions which should arise in this thread and I'll do my best to answer them.

I also managed to find a fairly good "white paper" on the topic of coding linked lists and have attached it below.

BigDog