2010-09-15 6 views
5

gcc 4.4.4 c89не может освободить память

У меня есть следующая функция, но я не могу освободить память. Сообщение, которое я получаю в Valgrind, подозревает функцию getline. Тем не менее, я свободен указатель файла в конце функции. Так не может быть.

У меня есть глобальный массив указателей на char 'candidates_names'. Однако я не выделил для этого никакой памяти.

Большое спасибо за любые советы,

сообщение, которое я получить в Valgrind, заключается в следующем.

HEAP SUMMARY: 
==4021==  in use at exit: 840 bytes in 7 blocks 
==4021== total heap usage: 22 allocs, 15 frees, 1,332 bytes allocated 
==4021== 
==4021== Searching for pointers to 7 not-freed blocks 
==4021== Checked 48,412 bytes 
==4021== 
==4021== 840 bytes in 7 blocks are still reachable in loss record 1 of 1 
==4021== at 0x4005BDC: malloc (vg_replace_malloc.c:195) 
==4021== by 0xAAE38D: getdelim (iogetdelim.c:68) 
==4021== by 0xAAADD2: getline (getline.c:34) 
==4021== by 0x804892B: load_candidates (candidate.c:61) 
==4021== by 0x8048686: main (driver.c:24) 

Мой исходный код:

#define NUMBER_OF_CANDIDATES 7 
static char *candidate_names[NAME_SIZE] = {0}; 

int load_candidates() 
{ 
    FILE *fp = NULL; 
    size_t i = 0; 
    ssize_t read = 0; 
    size_t n = 0; 
    char *found = NULL; 

    fp = fopen("votes.txt", "r"); 

    /* open the votes file */ 
    if(fp == NULL) { 
     fprintf(stderr, "Cannot open votes file [ %s ]\n", strerror(errno)); 
     return FALSE; 
    } 

    /* fill the structure with candidates */ 
    for(i = 0; i < NUMBER_OF_CANDIDATES;) { 
     read = getline(&candidate_names[i], &n ,fp); 
     if(read == -1) { 
      fprintf(stderr, "Cannot read candidate [ %d ] [ %s ]\n", 
        i, strerror(errno)); 
      candidate_names[i] = "Invalid candidate"; 
      i++; 
      continue; 
     } 
     /* Just ignore the key work in the text file */ 
     if(strcmp("CANDIDATES\n", candidate_names[i]) != 0) { 
      /* Find and remove the carriage return */ 
      found = strchr(candidate_names[i], '\n'); 
      if(found != NULL) { 
       /* Remove the carriage return by nul terminating */ 
       *found = '\0'; 
      } 
      i++; 
     } 
    } 

    fclose(fp); 

    return TRUE; 
} 

EDIT ========= ОСВОБОЖДЕНИЕ candidate_names ======

All heap blocks were freed -- no leaks are possible 
==4364== 
==4364== ERROR SUMMARY: 84 errors from 2 contexts (suppressed: 12 from 8) 
==4364== 
==4364== 42 errors in context 1 of 2: 
==4364== Invalid free()/delete/delete[] 
==4364== at 0x40057F6: free (vg_replace_malloc.c:325) 
==4364== by 0x8048A95: destroy_candidate (candidate.c:107) 
==4364== by 0x8048752: main (driver.c:44) 
==4364== Address 0x401e1b8 is 0 bytes inside a block of size 120 free'd 
==4364== at 0x40057F6: free (vg_replace_malloc.c:325) 
==4364== by 0x8048A95: destroy_candidate (candidate.c:107) 
==4364== by 0x8048752: main (driver.c:44) 
==4364== 
==4364== 
==4364== 42 errors in context 2 of 2: 
==4364== Invalid read of size 1 
==4364== at 0x400730E: strcmp (mc_replace_strmem.c:426) 
==4364== by 0x8048A7F: destroy_candidate (candidate.c:106) 
==4364== by 0x8048752: main (driver.c:44) 
==4364== Address 0x401e1b8 is 0 bytes inside a block of size 120 free'd 
==4364== at 0x40057F6: free (vg_replace_malloc.c:325) 
==4364== by 0x8048A95: destroy_candidate (candidate.c:107) 
==4364== by 0x8048752: main (driver.c:44) 


void destroy_candidate() 
{ 
    size_t i = 0; 
    for(i = 0; i < NUMBER_OF_CANDIDATES; i++) { 
     if(strcmp(candidate_names[i], "Invalid candidate") != 0) { 
      free(candidate_names[i]); 
     } 
    } 
} 

EDIT с кодом из main.c =====================

typedef struct Candidate_data_t { 
    size_t candidate_data_id; 
    Candidates_t *candidate; 
} Candidate_data; 

static Candidate_data* create_candidate_data(Candidates_t *candidate, size_t i); 
static void destroy_candidata_data(Candidate_data *cand_data); 

int main(void) 
{ 
    Candidates_t *candidate = NULL; 
    Candidate_data *cand_data[NUMBER_OF_CANDIDATES] = {0}; 
    size_t i = 0; 

    load_candidates(); 

    for(i = 0; i < NUMBER_OF_CANDIDATES; i++) { 
     candidate = create_candidates(i); 
     if(candidate == NULL) { 
      fprintf(stderr, "Cannot failed to initalize candidate [ %d ]\n", i); 
     } 

     /* Create array of candidates */ 
     cand_data[i] = create_candidate_data(candidate, i); 
     fill_candidates(cand_data[i]->candidate); 
    } 

    /* Display each candidate */ 
    for(i = 0; i < NUMBER_OF_CANDIDATES; i++) { 
     display_candidate(cand_data[i]->candidate); 
     printf("\n"); 
    } 

    for(i = 0; i < NUMBER_OF_CANDIDATES; i++) { 
     destroy_candidata_data(cand_data[i]); 
    } 

    return 0; 
} 

static Candidate_data* create_candidate_data(Candidates_t *candidate, size_t id) 
{ 
    Candidate_data *cand_data = NULL; 

    cand_data = malloc(sizeof *cand_data); 

    if(cand_data == NULL) { 
     fprintf(stderr, "Failed to allocate memory [ %s ]\n", 
       strerror(errno)); 

     return NULL; 
    } 
    cand_data->candidate_data_id = id; 
    cand_data->candidate = candidate; 

    return cand_data; 
} 

static void destroy_candidata_data(Candidate_data *cand_data) 
{ 
    destroy_candidate(cand_data->candidate); 
    free(cand_data); 
} 
+0

Ваше обновление с 'destroy_candidate()' все еще является ошибкой! Ошибка никогда не будет отображаться с вашей программой, но если вам когда-нибудь понадобится вызывать 'load_candidates()' более одного раза * (или больше до точки, если вам нужно 'getline()' к тому же указателю имя_компьютера [i]), * есть шанс, что библиотека попытается перераспределить неверный указатель. – pmg

ответ

2

getline выделяет память, которые вы храните в вашем candidate_names массиве указателей. Эти указатели не освобождаются. Когда вы закончите с ними, вы должны позвонить:

for(i = 0; i < NUMBER_OF_CANDIDATES; i++) 
{ 
    if (strcmp(candidate_names[i], "Invalid candidate") != 0) 
     free(candidate_names[i]); 
} 

Кроме того, этот массив должен быть объявлен как:

static char *candidate_names[NUMBER_OF_CANDIDATES]; 

И прямо перед вашим GetLine вам нужно:

candidate_names[i] = NULL; 

NAME_SIZE не требуется, поскольку эта память динамически распределяется, если вы не используете ее в другом месте для проверки ввода или чего-то еще.

+0

Я обновил свой вопрос. Я добавил функцию destroy_candidate() и освободил всю память. Однако я освобождаю всю выделенную память. «Никаких утечек невозможно». Тем не менее, я получаю некоторые ошибки «Недопустимый бесплатный». Благодарю. – ant2009

+0

Не могли бы вы разместить свой «главный»? –

+0

Я добавил свою главную функцию к отредактированному вопросу. Однако есть и другая функция. Я сохранил их на случай, если они понадобятся для поиска решения. Благодарю. – ant2009

7

Посмотрите на getline() man page.

Если * lineptr имеет значение NULL, то getline() будет выделять буфер для хранения строки, который должен быть освобожден программой пользователя. (В этом случае значение в * п игнорируются.)

В конце вашей программы, вам нужно перебрать ваш candidate_names массива и вызвать free на сторонних NULL записей, но в этом случае вы не должны делать candidate_names[i] = "Invalid candidate"; как @pmg указал в своем ответе, поскольку вы попытаетесь освободить строковый литерал.

Обратите также внимание на:

В качестве альтернативы, перед вызовом GetLine(), * lineptr может содержать указатель на таНос (3) -allocated буфер * N байт. Если буфер не достаточно велик, чтобы удерживать строку, getline() изменяет его размер с помощью realloc (3), обновляя * lineptr и * n при необходимости.

В любом случае, при успешном вызове, * lineptr и * n будут обновлены для отражения адреса буфера и выделенного размера соответственно.

4

getline() выделяет пространство для линии это только для чтения, называя malloc() для вас за кулисами. Вы сохраняете эти линейные буферы в массиве candidate_names, но никогда не освобождаете его. Утечка не указатель файла - вы освобождаете это просто отлично. Это строки, которые вы читаете из файла, которые необходимо освободить в другом месте, когда вы закончите использовать их.

-2

Я не думаю, что это правильно.

getline(&candidate_names[i], &n ,fp); 

Нет причин передавать указатель на целое число.

+0

getline - стандартная функция POSIX с прототипом 'ssize_t getline (char ** lineptr, size_t * n, поток FILE *);'. Правильно передается указатель на размер. – bdonlan

+0

кричит, почему-то я думал о Cline fstream getline. – Novikov

1

Я вижу, что у вас есть два разных макроса NUMBER_OF_CANDIDATES и NAME_SIZE. Похоже на неприятности.

+1

Это может быть так, но это не отвечает на вопрос ... Это, вероятно, было бы лучше как комментарий, а не ответ. – bdonlan

+1

Это не проблема *, но это проблема, особенно если 'NUMBER_OF_CANDIDATES' когда-либо растет больше, чем' NAME_SIZE'. –

+0

#define NAME_SIZE 80. Извините, что вышло. – ant2009

6

Что такое candidate_names? Это массив указателей.
Когда вы

candidate_names[i] = "Invalid candidate"; 

присвоить указатель на строку буквальным. Возможно, позже в программе вы хотите free. Это NO-NO!

В любом случае утеряно предыдущее значение candidate_names[i]. Если значение не было NULL, вы просто пропустили некоторую память.

+0

Я думал, что все в порядке. Не является ли строка литералом указателем? И я назначаю этот указатель на элемент массива указателей на char. Я не выделяю никакой памяти, поэтому нет необходимости освобождать. Это будет освобождено O/S, как только программа выйдет, как выделенная в стеке. Я прав? Благодарю. – ant2009

+0

good catch, +1 и я обновил свой ответ, чтобы отразить его –

+1

Предполагая, что имена ваших кандидатов достаточно длинные, попробуйте 'strcpy (кандидаты_имя [i],« Invalid кандидат »); вместо – pmg

1

Вы выделяете память внутри getline(). Вы никогда не освобождаете эту память. Это то, что вам говорит valgrind: у вас есть семь (== NUMBER_OF_CANDIDATES) блоков, которые вы не освободили.

Закрытие указателя файла не освобождает память, выделенную getline().

Вам нужно сделать что-то вроде этого в конце load_candidates():

for(int i = 0; i < NUMBER_OF_CANDIDATES; i++) 
{ 
    free(candidate_names[i]); 
} 

EDIT

В вашем редактирования вы освободив нулевые указатели. Попробуйте:

void destroy_candidate() 
{ 
    size_t i = 0; 
    for(i = 0; i < NUMBER_OF_CANDIDATES; i++) { 
     if ((candidate_names[i] != NULL) && (strcmp(candidate_names[i], "Invalid candidate") != 0)){ 
      free(candidate_names[i]); 
     } 
    } 
} 
Смежные вопросы