what's wrong with my code of linked list?

[questions?]

I am testing a problem with linked list.
I just do a lot of times: create a list, then free it.

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

struct element {
int index;
struct element *next;
};

void free_list(struct element * first){
struct element *current,*next;
current=first;
next=current->next;
while(next !=NULL){
printf("free the %d the element\n",current->index);
free(current);
current=next;
next=current->next;
}
free(current);
}

void print_list(struct element *first){
struct element *current;
current=first;
while(current !=NULL){
printf("the %d th element visited\n",current->index);
current=current->next;
}
}


struct element *create_list(void){
struct element *temp,*leading,*previous;
int i;
leading=malloc(sizeof(struct element));
leading->index=0;
previous=leading;

for(i=1;i<1000;i++){
temp=malloc(sizeof(struct element));
temp->next==NULL;
temp->index=i;
previous->next=temp;
previous=temp;
}
return leading;
}

int main(){
int i;
struct element *dummy;
for(i=0;i<2;i++){
dummy=create_list();
print_list(dummy);
free_list(dummy);
}
return 0;
}

******************************************************
What's wrong this this code? it is fine when I use for(i=0;i<1;i++) in
main().

Thanks

 

[Michael Rasmussen]

On Wed, 29 Mar 2006 23:28:26 -0800, questions? wrote:

A minor glitch

temp->next==NULL;

temp->next = NULL;

Try next time to add this option to your compile statement: -Wall
Then you would have received this warning:
test.c: In function 'create_list':
test.c:41: warning: statement with no effect

 

[Simple Simon]

(e-mail address removed) wrote...

 

[Keith Thompson]

I am testing a problem with linked list.
I just do a lot of times: create a list, then free it.

#############################################
# include <stdio.h>
# include <stdlib.h> [snip]

******************************************************
What's wrong this this code? it is fine when I use for(i=0;i<1;i++) in
main().

You're going to have to give us a hint. What is the program supposed
to do, and what does it actually do?

 

[Richard Bos]

I am testing a problem with linked list.
I just do a lot of times: create a list, then free it.

#############################################

[ Snip linked list code ]

******************************************************
What's wrong this this code? it is fine when I use for(i=0;i<1;i++) in
main().

What's your problem with it? It runs fine for me, and at first glance I
can't find anything wrong with it. The only nit I would pick is that
what you call "leading" is conventionally called the head node, but
that's not an actual error, it's a style issue.

Richard

 

[Richard Bos]

(e-mail address removed) wrote...

Argh! That's what's wrong with first glances: they miss the obvious...

Richard

 

[Marek Dzikiewicz]

void free_list(struct element * first){
struct element *current,*next;
current=first;
next=current->next;

This is illegal if the list is empty (next == NULL).

 

[Chris Torek]

Others already picked out various nits.

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

struct element {
int index;
struct element *next;
};

I usually put the forward link first, not that it matters much in

void free_list(struct element * first){
struct element *current,*next;
current=first;
next=current->next;
while(next !=NULL){
printf("free the %d the element\n",current->index);
free(current);
current=next;
next=current->next;
}
free(current);
}

To avoid redundant code and problems with freeing an empty list
and so on, I prefer to write:

void free_list(struct element *head) {
struct element *p, *next;

for (p = head; p != NULL; p = next) {
next = p->next;
free(p);
}
}

void print_list(struct element *first){
struct element *current;
current=first;
while(current !=NULL){
printf("the %d th element visited\n",current->index);
current=current->next;
}
}

Again, I would use a "for" loop; this time the match is so good
that we do not even need a separate "next" variable:

for (p = head; p != NULL; p = p->next)
printf("...\n", p->...);

struct element *create_list(void){

[contents snipped]

Here is where the BSD macros become particularly handy.

In general, I find that the list elements themselves might be
created with a "factory" (to steal a term more often applied to
OO-ish languages like C++). If the list is ordered in some
way, and/or if list-elements might be on multiple lists --
"every element in the entire system" but also "parent/child"
relationships with sibling and grandchild nodes and so forth,
and perhaps hash lists and timer queues and on and on -- I
can use the BSD macros to embed multiple "list" or "queue"
points inside each element.

Then, I can have multiple queue heads of various sorts, and
use TAILQ_APPEND, CIRCLEQ_INSERT_AFTER, and so on to do the
list manipulation.

Each type of queue has particular features and costs, as summarized
in the queue(3) manual (q.v.; use google).

 

[August Karlstrom]

(e-mail address removed) wrote...

One more item to add to the "Case Against C" list. ;-)


August

 

[Richard Tobin]

[/QUOTE]

One more item to add to the "Case Against C" list. ;-)

A reasonable compiler will warn you about this.

-- Richard

 

[August Karlstrom]

One more item to add to the "Case Against C" list. ;-)

A reasonable compiler will warn you about this.[/QUOTE]

Yes, you're right. It's unfortunate however that "no warnings" are
usually the default setting.


August

 

[Ben Pfaff]

Yes, you're right. It's unfortunate however that "no warnings" are
usually the default setting.

A reasonable programmer quickly learns to enable warnings

 

[questions?]

A reasonable programmer quickly learns to enable warnings

Thanks you all for a lot of good discussions.
I made an error in that line of ==.
I was trying to mimic what I did in my bigger program by giving a
simple example.
The error actually is not happening in the construct/free list process.

I found the package valgrind really helpful for finding errors.