0

i constantly get Segmentation fault after i added "ListRemoveHead" function to the my linked-list code...

if i comment out ListRemoveHead function the code works well, also it it seems i get the segmentation fault on the exit of main...

could you please help me find the bug ...

*updated with - gdb output, still segmentation fault.


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

typedef struct Person Person;

struct Person
{
    int     m_id;         /* Primary Key */
    char    m_name[128];
    int     m_age;
    Person* m_next;
};



/* create a person and give values */
Person* CreatePerson(int _id,char* _name,int _age)
{
    Person* ptrPerson;

    ptrPerson=malloc(sizeof(Person));

    ptrPerson->m_id = _id;
    strcpy(ptrPerson->m_name , _name);
    ptrPerson->m_age = _age;
    ptrPerson->m_next = NULL; 

    return ptrPerson;
}

/* insert a person to the top of the list */
Person* ListInsertHead(Person* _head ,Person* _p)
{
    if(NULL == _head)
    {
        return _p;
    }

    _p->m_next = _head;
    _head = _p;

    return _head;

}

Person* ListRemoveHead(Person* _head, Person** _item)
{
    if(NULL == _head)
    {
        return NULL;
    }

    *_item = _head;
    _head = _head->m_next;

    return _head;
}



void PrintList(Person* _head)
{
    if(NULL == _head)
    {
        return;
    }


    while(NULL != _head->m_next)
    {

        printf("id number %d, name is %s, age is %d\n", _head->m_id,_head->m_name,_head-> m_age);
        _head = _head->m_next;
        if(NULL == _head->m_next)
        {
            break;
        }
    }


}



int main()
{


    Person* ptrPerson[3];
    Person* ptrHead;
    Person** pptrItem ;

    ptrHead = malloc(sizeof(Person));

    ptrPerson[0] = CreatePerson(1,"mishel",4);
    ptrPerson[1] = CreatePerson(2,"peter",29);
    ptrPerson[2] = CreatePerson(3,"alex",32);



    ptrHead = ListInsertHead(ptrHead ,ptrPerson[0]);
    ptrHead =  ListInsertHead(ptrHead ,ptrPerson[1]);
    ptrHead = ListInsertHead(ptrHead ,ptrPerson[2]);


    ptrHead=ListRemoveHead(ptrHead,pptrItem);

    PrintList(ptrHead);
/*  printf("the removed item is:%d\n", (**pptrItem).m_id);*/

    return 0;
}

Starting program: /home/peter/Desktop/a.out

Breakpoint 1, CreatePerson (_id=1, _name=0x8048725 "mishel", _age=4)
    at linkedList.c:22
22      ptrPerson=malloc(sizeof(Person));
(gdb) n
24      ptrPerson->m_id = _id;
(gdb) n
25      strcpy(ptrPerson->m_name , _name);
(gdb) n
26      ptrPerson->m_age = _age;
(gdb) n
27      ptrPerson->m_next = NULL; 
(gdb) n
29      return ptrPerson;
(gdb) n
30  }
(gdb) n
main () at linkedList.c:97
97      ptrPerson[1] = CreatePerson(2,"peter",29);
(gdb) n

Breakpoint 1, CreatePerson (_id=2, _name=0x804872c "peter", _age=29)
    at linkedList.c:22
22      ptrPerson=malloc(sizeof(Person));
(gdb) 
24      ptrPerson->m_id = _id;
(gdb) 
25      strcpy(ptrPerson->m_name , _name);
(gdb) 
26      ptrPerson->m_age = _age;
(gdb) 
27      ptrPerson->m_next = NULL; 
(gdb) 
29      return ptrPerson;
(gdb) 
30  }
(gdb) 
main () at linkedList.c:98
98      ptrPerson[2] = CreatePerson(3,"alex",32);
(gdb) 

Breakpoint 1, CreatePerson (_id=3, _name=0x8048732 "alex", _age=32)
    at linkedList.c:22
22      ptrPerson=malloc(sizeof(Person));
(gdb) 
24      ptrPerson->m_id = _id;
(gdb) 
25      strcpy(ptrPerson->m_name , _name);
(gdb) 
26      ptrPerson->m_age = _age;
(gdb) 
27      ptrPerson->m_next = NULL; 
(gdb) 
29      return ptrPerson;
(gdb) 
30  }
(gdb) 
main () at linkedList.c:102
102     ptrHead = ListInsertHead(ptrHead ,ptrPerson[0]);
(gdb) 
103     ptrHead =  ListInsertHead(ptrHead ,ptrPerson[1]);
(gdb) 
104     ptrHead = ListInsertHead(ptrHead ,ptrPerson[2]);
(gdb) 
107     ptrHead=ListRemoveHead(ptrHead,pptrItem);
(gdb) 
109     PrintList(ptrHead);
(gdb) 
id number 2, name is peter, age is 29
id number 1, name is mishel, age is 4
112     return 0;
(gdb) 
113 }
(gdb) 
__libc_start_main (main=0x8048581 <main>, argc=1, argv=0xbffff0d4, 
    init=0x8048670 <__libc_csu_init>, fini=0x80486e0 <__libc_csu_fini>, 
    rtld_fini=0xb7fed180 <_dl_fini>, stack_end=0xbffff0cc) at libc-start.c:321
321 libc-start.c: No such file or directory.
(gdb) 


Program received signal SIGSEGV, Segmentation fault.
__run_exit_handlers (status=status@entry=0, listp=0xb7fbf3c4 <__exit_funcs>, run_list_atexit=run_list_atexit@entry=true) at exit.c:54
54  exit.c: No such file or directory.
(gdb) 






Program terminated with signal SIGSEGV, Segmentation fault.
The program no longer exists.
  • 1
    `*_item` points to a deallocated memory block. – barak manos Apr 08 '16 at 14:03
  • As a side note, the coding bug that I have mentioned above probably stems from a design error - you haven't really decided what the purpose of this function - change the head of the list and save the original head, or change the head of the list and release the allocated memory. – barak manos Apr 08 '16 at 14:06
  • That first argument to `CreatePerson()` is completely unnecessary and very confusing. – unwind Apr 08 '16 at 14:19
  • thanks @unwind i removed the first argument, still getting segmentation fault unfortunately... – Peter Berger Apr 08 '16 at 17:02
  • Possible duplicate of [Definitive List of Common Reasons for Segmentation Faults](http://stackoverflow.com/questions/33047452/definitive-list-of-common-reasons-for-segmentation-faults) – CodeMouse92 Apr 10 '16 at 00:37

2 Answers2

0

It looks that you need to swap this lines in ListRemoveHead function:

*_item = _head;
_head = _head->m_next;

because you return invalid pointer (freed) with _item.

Actually, later I noticed that you want to print removed item id. Then, you should redesign the code so it will instead instead just return id:

Person* ListRemoveHead(Person* _head, int* _item)
{
   ...
   *_item = _head->m_id;
   _head = _head->m_next;

just don't forget to check the return value in the main code:

ptrHead=ListRemoveHead(ptrHead, &item);
if (ptrHead) {
   PrintList(ptrHead);
   printf("the removed item is:%d\n", item);
} else {
   printf("list is empty\n")
}

Update

You also accessing uninitialized memory because you have in main

 ptrHead = malloc(sizeof(Person));

which is never initialized and becomes in the end of list. So, actually you don't need this line, just set initial value:

Person* ptrHead = NULL;
pmod
  • 10,450
  • 1
  • 37
  • 50
  • i removed the part i free *_item, my intension is to move the head of the list one node further, return a pointer to the new head to main while keeping the information of the removed node in *_item. if i comment out the function to remove the head all is fine , but if i keep it i get a segmentation fault after main has finished. – Peter Berger Apr 08 '16 at 16:34
  • @PeterBerger yes, this is another issue, see my update – pmod Apr 08 '16 at 18:52
0
  • You use a pointer to pointer pptrItem.

  • Then you make an assignment

    *_item = _head;
    
  • Till the above step things are fine.

  • You freed temp by

    free(temp); 
    /* This means that _item now stores the address of object whose  
     * contents are already freed.
     */
    
  • Then you access the the value stored in already freed memory in

    printf("the removed item is:%d\n", (**pptrItem).m_id);
    
  • But hey (**pptrItem).m_id doesn't exists at the moment. To simplify things a bit, replace *pptrItem by temp ( Here the temp is the memory chunk which is deallocated in the function Person* ListRemoveHead()), then it becomes (*temp).m_id and the result is segmentation fault.

Remedy

Change your ListRemoveHead to something like below

void ListRemoveHead(Person* _head) 
{
    int id;
    if(NULL == _head)
    {
        return NULL;
    }

    Person* temp;
    temp = _head;

    id=(*_head).m_id;

    _head = _head->m_next;

    free(temp);
    printf("The item removed is : %d\n",id);

}
sjsam
  • 21,411
  • 5
  • 55
  • 102
  • i removed the part i free *_item, my intention is to move the head of the list one node further, return a pointer to the new head to main while keeping the information of the removed node in *_item. if i comment out the function to remove the head all is fine , but if i keep it i get a segmentation fault after main has finished. – Peter Berger Apr 08 '16 at 16:37
  • What you mean by `remove the head`? Did you mean deallocate the space? `but if i keep it i get a segmentation fault ` The segmentation fault is due to the fifth point in this answer. It is suggested that you may deallocate the space for removed items especially if the list is big but you must not access any of the object members after the object is removed or `deallocated`. – sjsam Apr 08 '16 at 17:05