2014-02-07 5 views
-1

Ну в основном у меня были утечки памяти. Поэтому я хотел их исправить! добавлен free() в некоторых функциях. Запустите valgrind и получил успешное сообщение. Все утечки памяти исправлены или что-то вроде этого! И после этого у меня есть куча ошибок :(Я ДУМАЮ, что я положил free() правильно. Легко запутаться, потому что есть узел как указатель и узел как структура (смотрите в файле.h). Любая помощь оценивается . Спасибо. Извините, если этот вопрос прост. я новичок .....Обнаружение утечки памяти вызвало ошибки

код file.h

struct node { 
    int value; 
    struct node * next; 
}; 
typedef struct node List; 

int is_empty(List *); 
List *add_node(List *, int); 
List *remove_node(List *, int); 
List *create_node(int); 
char *tostring(List *); 

код в file.c

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

#define STRSIZE 128 /*assume string representation fits*/ 

/* Return true if the list h is empty 
* and false otherwise. 
*/ 
int is_empty(List *h) { 
    return h == NULL; 
} 

/* Create a list node with value v and return a pointer to it 
*/ 
List *create_node(int v) { 
    List *node = malloc(sizeof(List)); 
    free(node); 
    node->value = v; 
    node->next = NULL; 
    return node; 
} 

/* Insert a new node with the value v at the 
* front of the list. Return the new head of 
* the list. 
*/ 
List *add_node(List *h, int v) { 
    List *node = create_node(v); 
    node->next = h; 
    return node; 
} 

/* Remove the first node in the list h that 
* has the value v. Return the head of the 
* list. 
*/ 
List *remove_node(List *h, int v) { 
    List *curr = h; 

    /* Handle the cases when list is empty or the value 
    * is found at the front of the list */ 
    if (h == NULL) { 
     return h; 
    } else if (h->value == v) { 
     h = h->next; 
return h; 
    } 

    /* Look for the value in the list (keeping the pointer to 
    * the previous element so that we can remove the right 
    * element) */ 
    while (curr->next != NULL && curr->next->value != v) { 
     curr = curr->next; 
    } 

    if (curr->next == NULL) { /* v is not in list so do nothing */ 
     return h; 
    } else { /* curr->next->value == v */ 
     curr->next = curr->next->next; 
     return h; 
    } 
} 

/* Return a string representation of the list pointed to by h. 
*/ 
char *tostring(List *h) { 
    char *str= malloc(STRSIZE); 
    char num[4]; /* assume integer has max. four digits*/ 
    free(str); 
    str[0] = '\0'; 
    while (h != NULL) { 
     sprintf(num, "%d", h->value); 
     strncat(str, num, STRSIZE - strlen(str) - 1); 
     h = h->next; 
    } 
    return str; 
} 
+2

лучший способ исправить утечку памяти просто получить полный понять вашей программы. – moeCake

ответ

1

Возможно, вы хотите

temp = curr->next; 
curr->next = curr->next->next; 
free (temp); 

В текущем состоянии вашего кода remove_node является негерметичным (фактически утечка всего списка, если вы удалите все узлы).

Кроме того, чтобы остановить утечку памяти в функции tostring, вызывающая функция этой функции освобождает строку, возвращаемую tostring.

0

В функции remove_node

List *remove_node(List *h, int v) 
{ 
List *curr = h,*prev; 

/* Handle the cases when list is empty or the value 
* is found at the front of the list */ 
if (h == NULL) { 
    return h; 
} else if (h->value == v) { 
    h = h->next; 
    free(curr); 
    return h; 
} 

while (curr->next != NULL && curr->next->value != v) { 
    curr = curr->next; 
} 

if (curr->next == NULL) { /* v is not in list so do nothing */ 
    return h; 
} else { /* curr->next->value == v */ 
    prev = curr->next; 
    curr->next = curr->next->next; 
    free(prev); 
    return h; 
} 
} 

Вы должны освободить память, когда вы больше не хотите, что узел в вашем случае при удалении узла вы должны освободить память.

И в функции tostring не освобождайте память, которую вы только что выделили, освободите память в вызываемой функции после того, как str больше не используется или не требуется.

+0

Ну, у меня еще есть утечка памяти где-то .... Нет никаких ошибок, хотя .... @ Karthik – Manuel

+0

утечка памяти происходит всякий раз, когда вы назначаете память с использованием динамического выделения (malloc, realloc), поэтому в вашей проверке, где все, что вы назначили во время выполнения (динамическое), и вы должны все это делать. только место, где происходит утечка памяти, - это строка, поэтому перед тем, как вы освободите узел, свободную строку, затем узел. – const

1

В вашем create_node(), почему у вас есть бесплатный()? Вы должны удалить этот бесплатный().

В вашем remove_node() вам всегда нужно вернуть голову. Я думаю, что следующий рекурсивный метод должен работать. Рекурсивная функция найдет удаляемый узел и вернет следующий узел. Он возвращает тот же узел, если ничего не удалено. Сохраняет рекурсию до тех пор, пока не будет найден узел со значением или конец списка. Главное, чтобы проверить это, он работает, когда

  1. Удаление узла головки
  2. пустой список
  3. Удаление последнего узла
  4. Извлечении средний узел

Следующий код не испытано :-).

Вы упомянули, что у вас все еще есть утечка памяти. Как уже упоминалось Karthik, ваша функция toString() выделяет память, и ее вызывающая сторона может освободить ее. Поэтому убедитесь, что каждый раз, когда вы вызываете функцию toString(), вызывается также free(). В противном случае вы будете утечки памяти.

List *remove_node(List *h, int v) { 
    List *retval = remove_recurse(h,v); 

    // if retval is different from head, means head is removed. return new head. 
    // Otherwise return old head. 
    return (retval!=h) ? retval : h; 
} 

List *remove_recurse(List *node, int v) { 
    if(node==NULL) return NULL; 

    List *retval = node; // default return current node 

    if(node!=NULL && node->value==v) { // need to remove node 
     retval = node->next; // return the next node 
     free(node); // delete node 
    } 
    else { 
     List *temp = remove_recurse(node->next,v); 
     // if next node was deleted, point to new node 
     if(node->next!=temp) node->next=temp; 
    } 

    return retval; 
} 
1

С этим

List *node = malloc(sizeof(List)); 
free(node); 

вы выделить узел (List), то пусть узловую точку на ней, то вы свободны, что узел указывает на так после свободной он указывает на некоторое незанятое пространство где-то в памяти, то вы начнете присваивание этой памяти:

node->value = v; 
node->next = NULL; 

, который вызывает неопределенное поведение, это, безусловно, неправильно, но не будете обнаружены компилятором.

удалить free(node)

List *create_node(int v) 
{ 
    List *node = malloc(sizeof(List)); 
    node->value = v; 
    node->next = NULL; 
    return node; 
} 

это более читаемым, если вы сохранить название в структуры такой же, как ЬурейеЕ т.е.

typedef struct node {...} node; 

вместо создания нового псевдонима, используйте вместо этого лучше переменной имя, например

node* listStartOfNodes = NULL; // always initialize 
1

1, эта функция неправильная.

List *create_node(int v) { 
    List *node = malloc(sizeof(List)); 
    free(node); 
    node->value = v; 
    node->next = NULL; 
    return node; 
} 

Почему вы работаете free(node)? пожалуйста, удалите это free(node);. И та же ошибка в этой функции, пожалуйста, удалите free(str);

char *tostring(List *h) { 
    char *str= malloc(STRSIZE); 
    char num[4]; /* assume integer has max. four digits*/ 
    free(str); 
    str[0] = '\0'; 
    while (h != NULL) { 
     sprintf(num, "%d", h->value); 
     strncat(str, num, STRSIZE - strlen(str) - 1); 
     h = h->next; 
    } 
    return str; 
} 

2, вы должны изменить эту функцию:

List *remove_node(List *h, int v) { 
    List *curr = h; 
    List* freeNode = NULL; 
    /* Handle the cases when list is empty or the value 
    * is found at the front of the list */ 
    if (h == NULL) { 
     return h; 
    } else if (h->value == v) { 
     freeNode = h; 
     h = h->next; 
     free(freeNode); 
     return h; 
    } 

    /* Look for the value in the list (keeping the pointer to 
    * the previous element so that we can remove the right 
    * element) */ 
    while (curr->next != NULL && curr->next->value != v) { 
     curr = curr->next; 
    } 

    if (curr->next == NULL) { /* v is not in list so do nothing */ 
     return h; 
    } else { /* curr->next->value == v */ 
     freeNode = curr->next; 
     curr->next = curr->next->next; 
     free(freeNode); 
     return h; 
    } 
} 
Смежные вопросы