Singly Linked List struct insert/delete multiple free
Been looking at this code for too long and I am getting gloomy any chance of figuring it out by myself has been lost :( anyone can tell me where am I being stupid? 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 name[30];
long ID;
char address[40];
char city[20];
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(t开发者_JS百科emp -> 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;
}
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.
Not having read all of it, what are the first lines of delete
supposed to do?
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?
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));
精彩评论