Singly Linked List struct insert/delete multiple free?

Up vote 0 down vote favorite share g+ share fb share tw.

I just don't understand where I am double freeing or possibly allocating incorrectly (which I must bee doing but yeah). I keep getting * glibc detected * free(): invalid next size Am I actually freeing more than I need to or am I just not allocating what I need to allocate in the first place. --sorry for bad indentation can't get this editor to indent correctly I have structures: typedef int boolean; typedef char * String; typedef struct { char name30; long ID; char address40; char city20; int age; }Employee; typedef struct node { Employee *anEmployee; struct node *next; }NODE; typedef struct { NODE *head, *tail; }SLL; insert function--SLL (Singly Linked List) void insert(SLL *list, Employee e){ printf("insert"); NODE *temp, *current; temp = (NODE *)malloc(sizeof(NODE)); assert(temp!

= NULL); temp -> anEmployee = (Employee *)malloc(sizeof(Employee *)); assert(temp -> anEmployee! = NULL); strcpy(temp -> anEmployee -> name, e. Name); temp -> anEmployee -> ID = e.

ID; strcpy(temp -> anEmployee -> address, e. Address); strcpy(temp -> anEmployee -> city, e. City); temp -> anEmployee -> age = e.

Age; if (list -> head == NULL) { /* list is empty */ list -> head = list -> tail = temp; return; } else { // list is not empty list -> tail -> next = temp; list -> tail = temp; } } deleting and freeing memory in delete function as such boolean delete(SLL *list, String str){ printf("delete"); NODE *temp, *temp2; if (list -> head == NULL) return FALSE; // list is empty temp = list -> head; while ((temp! = NULL) && (strcmp(temp -> anEmployee -> name , str)! = 0)) temp = temp -> next; if (temp == NULL) return FALSE; // str is not found in the list // now temp points to the NODE with str in it.

Let us delete it // from the list if ( list -> head == temp) { // temp points to the first NODE if (temp -> next == NULL) { // temp points to the only NODE list -> head = list -> tail = NULL; free(temp -> anEmployee); free(temp); return TRUE; } // temp points to the first NODE but it is not the only NODE list -> head = temp -> next; free(temp -> anEmployee); free(temp); return TRUE; } if (temp -> next == NULL) { // temp points to the last NODE temp = list -> head; temp2 = list -> head -> next; while(temp2 - > next! = NULL){ temp = temp->next; temp2 = temp2 ->next; } list -> tail = temp ; list -> tail -> next = NULL; free(temp2 -> anEmployee); free(temp2); return TRUE; } // temp points to some NODE in the middle of the list temp2 = temp -> next; while(temp2 - > next! = NULL){ temp ->anEmployee = temp2 - > anEmployee // temp = temp->next; temp2 = temp2 ->next; } temp ->anEmployee = temp2 - > anEmployee list -> tail = temp ; list -> tail -> next = NULL; free(temp2 -> anEmployee); free(temp2); return TRUE; } c struct malloc free link|improve this question edited Apr 23 '11 at 4:58 asked Apr 23 '11 at 4:49WilsonJP32.

For the indentation: search-and-replace tabs with four spaces. – Potatoswatter Apr 23 '11 at 4:51.

First, in insert, You're allocating temp -> anEmployee = (Employee *)malloc(sizeof(Employee *)); which only allocates enough memory to hold an Employee pointer, not an entire Employee structure. You should allocate a block the size of sizeof(Employee) for temp->anEmployee. Your calls to free make sense insofar as you do want to free someNode->anEmployee and someNode to completely clean up the memory occupied by an individual node.

You could simplify your delete implementation as follows: boolean delete(SLL* list, String str) { NODE* temp = list->head, *prev = NULL; while(temp! = NULL && strcmp(temp->name, str)! = 0) { prev = temp; temp = temp->next; } if(temp == NULL) return FALSE; if(prev!

= NULL) prev->next = temp->next; if(list->head == temp) list->head = temp->next; if(list->tail == temp) list->tail = temp->next; free(temp->anEmployee); free(temp); return TRUE; } By tracking the node which precedes your find, if any, you can avoid all of the nasty special cases and reduce the core list update to three simple conditional assignments.

I don't see the case where you are deleting a node in the middle of the list, however that does simplify the rest of my code greatly. --nvm I see. – WilsonJP Apr 23 '11 at 5:23.

NODE *temp, *temp2; // temp is uninitialized if ( list -> head == temp) { // temp points to the first NODE I can't tell what delete is supposed to be deleting. It appears that the str argument is unused. Do you want to search for a particular record based on str, set temp to point to it, and then proceed with the code as shown?

Hhm I removed the part where it is searches though the list for str and if it is found it makes temp point to that node, temp2 is needed for the some of the delete cases (should have probably used switch). – WilsonJP Apr 23 '11 at 4:56 @Wilson: Aha, I guessed it! – Potatoswatter Apr 23 '11 at 4:56 @Wilson: Not to be too argumentative, but this kind of code is just as efficient and far easier to write in C++.

– Potatoswatter Apr 23 '11 at 4:59 it is a unix class ^^ C programming – WilsonJP Apr 23 '11 at 5:00 @Wilson: Again not to be too argumentative, but Unix is just a family of operating systems, which all work quite well with C++. This problem is clearly in the application programming domain. Even if it were part of the OS kernel, even more reason to use the right tool for the job to achieve reliability.

Mac OS X is first-class UNIX and uses C++ in the kernel and objective-C in userspace—so what? – Potatoswatter Apr 23 '11 at 5:15.

In insert(), when you allocate temp->anEmployee, you're only allocating enough space for the pointer, not the full Employee. This line: temp -> anEmployee = (Employee *)malloc(sizeof(Employee *)); should be: temp -> anEmployee = (Employee *)malloc(sizeof(Employee)).

Great thanks works. – WilsonJP Apr 23 '11 at 5:01.

I cant really gove you an answer,but what I can give you is a way to a solution, that is you have to find the anglde that you relate to or peaks your interest. A good paper is one that people get drawn into because it reaches them ln some way.As for me WW11 to me, I think of the holocaust and the effect it had on the survivors, their families and those who stood by and did nothing until it was too late.