Thread: Another List Q - Seg fault on head-tail list

  1. #1
    Registered User
    Join Date
    May 2006
    Posts
    22

    Another List Q - Seg fault on head-tail list

    And I am completely stumped as to why i am getting it.. for all intents and purposes, I cannot see a flaw (by eyeballing the code).



    Code:
    typedef struct list {
    	int number;
    	struct list *next;
    } node;
    
    node *addlist(node *head, node *tail, int number)
    {
    	node *temp;
    
    	temp = calloc(1, sizeof(node));
    	temp->number = number;
        
        tail->next = temp;
        tail = temp;
        
        if ( head == NULL )
        {
            head = tail;
        }
        
        nodecount++;
        
        return tail;
    
    }
    
    void printlist( node *head, node *tail )
    {
    
    	node *point;
    	point = head;
    
    	while (point)
    	{
    		printf("Number: %d\n", point->number);
    		point = point->next;
    	}
    
    }
    
    
    int main( int argc, char *argv[] ) 
    {
    
    	node *head = NULL;
        node *tail = NULL;
    
    	if ( arguementscheck(argc) != 1 )
    	{
    		error(1);
    	}
    
    	if ( readcheck(argv[1]) != 1 )
    	{
    		error(2);
    	}
    
    
    	tail = addlist(head, tail, 1);
    	tail = addlist(head, tail, 3);
    	tail = addlist(head, tail, 5);
    	tail = addlist(head, tail, 10);
    
    	printf("Inserted %d nodes into my list.\n", nodecount);
    
    	printlist(head, tail);
    
    	/*
    		No errors encountered, all tasks completed.
    		Exit the program.
    	*/
    
    	stdexit();
    
    }
    Last edited by JimpsEd; 05-09-2006 at 05:07 AM.

  2. #2
    ATH0 quzah's Avatar
    Join Date
    Oct 2001
    Posts
    14,826
    Well clearly that's not all of your code, because you have a 'nodecount' variable which is apparently global, that isn't shown. How about putting some printf statements scattered around in your code, so you can see how far it gets? Also, you have a bad habbit of not setting your 'next' pointer to NULL. calloc doesn't guarintee pointers end up NULL. It likely ends up that way, but there is at least one implementation, which NULL does not equal all bits zero.


    Quzah.
    Hope is the first step on the road to disappointment.

  3. #3
    Registered User
    Join Date
    May 2006
    Posts
    22
    Ok, here is my code again.

    It falls over at;
    Code:
    	/* DEBUG */ printf("tail set to temp.\n");
        tail = temp;
    with a seg fault.

    Code:
    typedef struct list {
    	int number;
    	struct list *next;
    } node;
    
    int nodecount = 0;
    
    node *addlist(node *head, node *tail, int number)
    {
    
    	node *temp;
    	/* DEBUG */ printf("Created new temp node, calloc. Setting temp->number to number.\n");
    
    
    	temp = calloc(1, sizeof(node));
    	temp->number = number;
        
    	/* DEBUG */ printf("tail->next set to temp.\n");
        tail->next = temp;
    
    	/* DEBUG */ printf("tail set to temp.\n");
        tail = temp;
        
        if ( head == NULL )
        {
    		/* DEBUG */ printf("head == NULL, so set head = tail\n");
            head = tail;
        }
        
        nodecount++;
        
    	/* DEBUG */ printf("Return tail.\n Done.\n\n.");
        return tail;
    
    }
    
    
    
    void printlist( node *head, node *tail )
    {
    
    	node *point;
    	point = head;
    
    	while (point)
    	{
    		printf("Number: %d\n", point->number);
    		point = point->next;
    	}
    
    }
    
    
    
    int main( int argc, char *argv[] ) 
    {
    
    	node *head = NULL;
    	node *tail = calloc(1, sizeof(node));;
    
    	/* DEBUG */ printf("Created nodes head and tail.\n");
    
    	if ( arguementscheck(argc) != 1 )
    	{
    		error(1);
    	}
    
    	if ( readcheck(argv[1]) != 1 )
    	{
    		error(2);
    	}
    
    	/* DEBUG */ printf("Passing head,tail,1 to the addlist function.\n");
    	tail = addlist(head, tail, 1);
    	tail = addlist(head, tail, 3);
    	tail = addlist(head, tail, 5);
    	tail = addlist(head, tail, 10);
    
    	printf("Inserted %d nodes into my list.\n", nodecount);
    	
    	printlist(head, tail);
    
    	/*
    		No errors encountered, all tasks completed.
    		Exit the program.
    	*/
    
    	stdexit();
    
    }
    Last edited by JimpsEd; 05-09-2006 at 10:25 AM.

  4. #4
    Gawking at stupidity
    Join Date
    Jul 2004
    Location
    Oregon, USA
    Posts
    3,218
    When the first node is created, head and tail are both NULL so when you do:
    Code:
        tail->next = temp;
    it barfs. tail has to point to some valid memory before you try dereferencing it.
    If you understand what you're doing, you're not learning anything.

  5. #5
    Registered User
    Join Date
    May 2006
    Posts
    22
    Ok, got that fixed. Now my printlist function isn't printing anything...

  6. #6
    Gawking at stupidity
    Join Date
    Jul 2004
    Location
    Oregon, USA
    Posts
    3,218
    That's because head isn't being updated in main(). Everything in C is passed by value. So when you do addlist(head, etc.) you're passing the value that the head pointer contains to addlist(). So head gets updated in the addlist() function because it has it's only local pointer but it remains the same in main().

    What you need to do is pass a pointer to head to addlist() like:
    Code:
    tail = addlist(&head, tail, 1);
    And addlist() would then need to be changed to treat head as a pointer to a pointer (node **head). To change head then, you'd need to do *head = tail inside your addlist() function.
    If you understand what you're doing, you're not learning anything.

  7. #7
    Registered User
    Join Date
    May 2006
    Posts
    22
    Hi;

    Thanks again for your help.

    I now get the following warning, and still no printing:

    --------------------Configuration: Parser - Win32 Debug--------------------
    Compiling...
    Parser.c
    C:\Parser.c(147) : warning C4047: '=' : 'struct list ** ' differs in levels of indirection from 'struct list *'

    Parser.obj - 0 error(s), 1 warning(s)

  8. #8
    Gawking at stupidity
    Join Date
    Jul 2004
    Location
    Oregon, USA
    Posts
    3,218
    What does your addlist() function look like now?
    If you understand what you're doing, you're not learning anything.

  9. #9
    Registered User
    Join Date
    Nov 2005
    Posts
    95
    Quote Originally Posted by quzah
    calloc doesn't guarintee pointers end up NULL. It likely ends up that way, but there is at least one implementation, which NULL does not equal all bits zero.
    Quzah.

    So we must always test pointers against NULL ?
    Code:
    char  buf[] = "abc";
    char  *p;
    
    p = strchr(buf, 'z');
    
    /* This is wrong ? */
    if ( ! p )
       printf("Failed to find z\n");
       
    /* We must always test it with NULL ? */
    if (  p == NULL )
       printf("Failed to find z\n");

  10. #10
    Registered User
    Join Date
    May 2006
    Posts
    22
    Ok got it working Except for one problem now. The printlist() function prints a "0" at the first print.. but I can't see why:

    Code:
    typedef struct list {
    	int number;
    	struct list *next;
    } node;
    
    node *addlist( node *tail, node *(*head), int number )
    {
        node *temp;
    
        if( (temp = calloc(1, sizeof(node))) == NULL)
        {
            error(3);
        }
    
        temp->number = number;
        temp->next = NULL;
    
    	if ( *head == NULL )
    	{
    		*head = tail;
    	}
    
        if ( tail ) 
    	{
    		tail->next = temp;
    		tail = temp;
    	}
    
    	nodecount++;
    
        return tail;
    }
    
    
    
    void printlist( node *p )
    {
    
    	node *point;
    	point = p;
    
    	while ( point )
    	{
    		printf("Number: %d\n", point->number);
    		point = point->next;
    	}
    
    }
    
    
    
    int main( int argc, char *argv[] ) 
    {
    
    	node *head = NULL;
    	node *tail = calloc(1, sizeof(node));
    
    	if ( arguementscheck(argc) != 1 )
    	{
    		error(1);
    	}
    
    	if ( readcheck(argv[1]) != 1 )
    	{
    		error(2);
    	}
    
    	tail = addlist(tail, &head, 1);
    	tail = addlist(tail, &head, 3);
    	tail = addlist(tail, &head, 5);
    	tail = addlist(tail, &head, 10);
    
    	printf("Inserted %d nodes into my list.\n", nodecount);
    	
    	printlist(head);
    
    	/*
    		No errors encountered, all tasks completed.
    		Exit the program.
    	*/
    
    	stdexit();
    
    }
    Inserted 4 nodes into my list.
    Number: 0
    Number: 1
    Number: 3
    Number: 5
    Number: 10
    Cleanly exiting program.
    Last edited by JimpsEd; 05-09-2006 at 03:31 PM.

  11. #11
    Gawking at stupidity
    Join Date
    Jul 2004
    Location
    Oregon, USA
    Posts
    3,218
    You've got a problem here:
    Code:
    	if ( *head == NULL )
    	{
    		*head = tail;
    	}
    
        if ( tail ) 
    	{
    		tail->next = temp;
    		tail = temp;
    	}
    In the first check, tail is still NULL on the first call. You should be setting head to temp, not tail.

    In the second check, tail only ever changes if tail is not NULL to begin with. tail is NULL throughout your entire program the way you have it set up. Try this instead:
    Code:
    if(!*head)
      *head = temp;
    if(tail)
      tail->next = temp;
    tail = temp;  // This should ALWAYS get set. Not just if tail is not NULL
    If you understand what you're doing, you're not learning anything.

  12. #12
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,656
    > So we must always test pointers against NULL ?
    No, that's not what he meant.
    calloc sets all the bits in the allocated memory to zero, so if you have something like
    Code:
    typedef struct list {
    	int number;
    	struct list *next;
    } node;
    
    node *p = calloc ( 1, sizeof(node) );  // try and make p->next NULL by filling with zeros
    if ( p->next == NULL ) // may fail on some machines.
    
    // You must do this after you call calloc to initialise the data properly
    p->next = NULL
    This of course makes calloc pretty useless for portable code, since you can only guarantee behaviour for integral types. floats, doubles and pointers cannot be usefully initialised with calloc.
    http://c-faq.com/malloc/calloc.html


    Testing the return result of allocation functions with either, it doesn't matter.
    if ( ! p )
    if ( p == NULL )
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Linked List & Seg Fault
    By lord in forum C++ Programming
    Replies: 6
    Last Post: 06-01-2008, 12:39 PM
  2. Pleas take a look & give a critique
    By sh3rpa in forum C++ Programming
    Replies: 14
    Last Post: 10-19-2007, 10:01 PM
  3. Replies: 5
    Last Post: 11-04-2006, 06:39 PM
  4. Contest Results - May 27, 2002
    By ygfperson in forum A Brief History of Cprogramming.com
    Replies: 18
    Last Post: 06-18-2002, 01:27 PM
  5. singly linked list
    By clarinetster in forum C Programming
    Replies: 2
    Last Post: 08-26-2001, 10:21 PM