Thread: Segmentation fault

Typedef should be used to make code clearer, not to obfuscate it.

Code:

struct node 
{
    struct node *prev, *next;
    int data;
};

void insert( struct node *toInsert, struct node **list )
{
    toInsert->prev = NULL;
    toInsert->next = *list;
    if( *list != NULL )
         *list->prev = toInsert;
    *list = toInsert;
}

See how much cleaner that is? You could easily modify this for your needs, by making it take the info to put in a new node and then insert it.

Quzah.

Originally posted by NightWalker
I think the code I have written is very clear for me...

...I didn't learn very details about C, so I do not understand the code you wrote for me. I am wondering why my code cannot work, it seems that there is no logical problem with that? could u explain it to me?

See, that's the problem. Mine is clear to me, while yours is not, because you overly use typedef to death. I have no idea what any of your types are, and can only guess. While looking at mine, it's immediately clear what each type is for. Personal preference I suppose, but that's one of the reasons I hate typedef.

It in itself isn't bad, but people seem to typedef the stupidest things. Such as pointers as another type. This annoys me to no end. But again, personal preference.

I'll venture a guess at yours:

Code:

void
        Insert( ElementType X, List L, Position P )

Ok, ElementType is probably an int or something, no biggie here.
List is likely a structure, my guess is it's something like:

Code:

typedef struct list
{
    struct node *list;
} List;

Or perhaps it contains two pointers. Something equally stupid...er...useful.
Position is probably a "node".

So looking at your code, I'll comment as I go:

Code:

void Insert( ElementType X, List L, Position P )
{
	  Position TmpCell, PORNEXT;
	  
	    TmpCell = malloc( sizeof( struct Node ) );
	    if( TmpCell == NULL )
	      FatalError( "Out of space!!!" );
	    /* I assume FatalError exits. */

	    PORNEXT = P->Next;
	    /* Bad use of caps. I doubt you
	        need this variable at all. */

	    TmpCell->Element = X;
	    TmpCell->Next = PORNEXT;
	    /* Why didn't you just make TmpCell->Next
	        point to 'P->Next'? */

	    P->Next = TmpCell;
	    /* Wrong. Look at what you have:
	        PO = P->Next
	        TmpCell->next = PO
	        P->Next = TmpCell
	        You have a nice little loop here.
	        Everyone points at everyone.
	        There really is no need for me to
	        debug this further at this point. */

	    TmpCell->Last = P;
	    PORNEXT->Last = TmpCell;  // THIS LINE
        }

You have an unneeded variable 'PORNEXT', and you end up making everyone point at everyone else in a loop. Probably not what you want.

Anyway, as the main point of the post, your using typedefs all over the place really don't give me much to go on. All I can do is guess what data types you have and what members they contain.

So in short, if you really want help in the future, and you insist on using typedefs and the like, you'll want to be sure and post all of the required information, such as structure definitions and typedefs.

Quzah.

Thanks, quzah. You are right, I'd better show my purpose and make my code more clear next time. I found the problem when I was brushing my teeth this morning, the fault is: while I am trying to insert an element into the list, the 'Position P' I passed to the function may be the last node of the list, at first, the list had only one node: The Header(also the last node). I use PORNEXT to point the next node of that one, is actually NULL, so, the assignment 'PORNEXT->Last = TmpCell' will fail without doubt. I modified the code and it works.
Quzah, thank you for your help .
This is the insert funtion I modified:

Code:

        void
        Insert( ElementType X, List L, Position P )
        {
	    Position TmpCell, PORNEXT;   // original P->Next: PORNEXT
	    TmpCell = malloc( sizeof( struct Node ) );
	    if( TmpCell == NULL )
	      FatalError( "Out of space!!!" );
	    if(P->Next == NULL)
	      {
		TmpCell->Element = X;
		TmpCell->Next = P->Next;
		TmpCell->Last = P;
		P->Next = TmpCell;
	      }
	    else
	      {
		PORNEXT = P->Next;
		TmpCell->Element = X;
		TmpCell->Next = P->Next;
		TmpCell->Last = P;
		P->Next = TmpCell;
		PORNEXT->Last = TmpCell;
	      }
        }

The insertion sort using linked-list:

Code:

void
InsertionSortList(List L)
{
  int num, status;
  Position P, P1, P2;
  ElementType temp;
  FILE *inp;
  inp = fopen("data10000.txt", "r");
  for(P = Header(L); status != EOF; P = P->Next)
    {
      status = fscanf(inp, "%d", &num);
      Insert(num, L, P);
    }
   for(P1 = Header(L)->Next; P1->Next != NULL; P1 = P1->Next)
     {
       temp = P1->Element;
       for(P2 = P1; (P2->Last != NULL) && (P2->Last->Element > temp); P2 = P2->Last)
	 {
	   P2->Element = P2->Last->Element;
	 }
       P2->Element = temp;
     }
}