开发者

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));
0

上一篇:

下一篇:

精彩评论

暂无评论...
验证码 换一张
取 消

最新问答

问答排行榜