Segmentation Fault

[diosoxido]

(this program adds a node at the bottom of a list using virtual memory)
I have a problem running my code on a (Unix) SUN machine, it compiles fine but with this part of the code i got a "Segmentation Fault". Works fine on my PC if i run it with BORLAND C.

This is part of the code i think is giving trouble!

void add( struct node *new ){
if( head == NULL )
head = new;
end->next = new;/*here*/
new->next = NULL;
end = new;
}

 

[marsd]

Several things to ask yourself:

1) If new has been dynamically alloc'd does it
have any memory? If not how are you going to add it to anything?

2) Where is my pointer to node for the head and end
nodes?? The end node looks bad to me, I don't see it's
importance in this snippet. Why do you assign both head
and end to new after beating around with end->next?

If I want a new member and node is doubly linked
I could do something like:

struct node *linkAdd(struct node *prev) {
struct node *inside = malloc(sizeof(struct node));

if (inside) {
inside->prev = prev;
inside->next = NULL;
other values assigned here..
return inside;
} else {
perror("Malloc() in linkAdd()"
return NULL:
}
}

 

[LF28]

What happens if head is zero? More precisely: what value does end point to?
In case the list is empty (head == 0) you use end (end->next = new). But is end a valid pointer in this case?

Maybe you wanted to do something like this:

void add(struct node* new ) {
new->next = 0;
if( head == 0 )
{
head = new;
}
else
{
end->next = new;
}
end = new;
}

This will make sure end is not used if the list is empty.

regards,
Tobi

 

[brudnakm]

I would recommend the following code. It works whether the list is empty or not. The only drawback is that it appends new items to the font of the list. It is clean and easy and if order does not matter it is the preferred method.

static head = NULL ;
void addToFront( struct node *new ){
   new->next = head ;
   head = new ;
}

Otherwise you could add to the end of the list as follows:

static head = NULL ;
static end ;
void addToEnd( struct node *new ){
   if( head == NULL ){ 
       /* first time */
       head = new;
       new->next = NULL ; 
   } else {  
      /* typical case */
      new->next = end->next ;
      end->next = new;
   }
   end = new;
}

This is worse because you have to handle special case of the empty list. It makes for ugly code.

Hope this helps.

Mark.