не може да освободи памет

gcc 4.4.4 c89

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

Имам глобален масив от указатели към char 'candidate_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;
}

РЕДАКТИРАНЕ ========= ОСВОБОЖДАВАНЕ на имена на кандидати ======

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]);
        }
    }
}

РЕДАКТИРАНЕ с код от 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);
}

person ant2009    schedule 15.09.2010    source източник
comment
Вашата актуализация с destroy_candidate() все още е грешка! Грешката никога няма да се покаже с вашата програма, но ако някога трябва да извикате load_candidates() повече от веднъж (или повече по същество, ако трябва да getline() към същия указател за кандидат_име[i]) има шанс библиотеката да се опита да преразпредели невалиден указател.   -  person pmg    schedule 15.09.2010


Отговори (7)


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 не е необходимо, защото тази памет е динамично разпределена, освен ако не я използвате другаде за валидиране на въвеждане или нещо подобно.

person Karl Bielefeldt    schedule 15.09.2010
comment
Актуализирах въпроса си. Добавих функция към destroy_candidate() и освобождавам цялата памет. Въпреки това освобождавам цялата разпределена памет. „Не са възможни течове“. Въпреки това получавам някои грешки „Невалидно безплатно“. Благодаря. - person ant2009; 15.09.2010
comment
Добавих основната си функция към редактирания въпрос. Там обаче има друга функция. Запазих ги, в случай че са необходими за намиране на решение. Благодаря. - person ant2009; 16.09.2010
comment
Вашето destroy_candidate обаждане не съответства на определението, което публикувахте по-рано. - person Karl Bielefeldt; 16.09.2010
comment
@Karl. Това беше пълният код. Въпреки това направи много повече. Току-що публикувах пример в поредния въпрос. Ще се опитам да оправя това сам. Изглежда просто ще трябва да го разглобя. Благодаря. - person ant2009; 16.09.2010

Разгледайте getline() справочната страница.

Ако *lineptr е NULL, тогава getline() ще разпредели буфер за съхраняване на реда, който трябва да бъде освободен от потребителската програма. (В този случай стойността в *n се игнорира.)

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

Обърнете внимание и на:

Алтернативно, преди да извика getline(), *lineptr може да съдържа указател към malloc(3)-разпределен буфер с размер *n байта. Ако буферът не е достатъчно голям, за да побере реда, getline() го преоразмерява с realloc(3), като актуализира *lineptr и *n, ако е необходимо.

И в двата случая, при успешно повикване, *lineptr и *n ще бъдат актуализирани, за да отразят съответно адреса на буфера и разпределения размер.

person Gregory Pakosz    schedule 15.09.2010

Какво е candidate_names? Това е масив от указатели.
Когато го направите

candidate_names[i] = "Invalid candidate";

присвоявате указателя на низов литерал. Може би по-късно в програмата искате да го free. Това е НЕ-НЕ!

Във всеки случай предишната стойност на candidate_names[i] се губи. Ако стойността не е NULL, просто сте изпуснали малко памет.

person pmg    schedule 15.09.2010
comment
Мислех, че е напълно добре. Стринговият литерал не е ли указател? И аз присвоявам този указател на an елемент от масива от указатели към char. Не заделям памет, така че няма нужда да освобождавам. Това ще бъде освободено от O/S, след като програмата излезе, както е разпределена в стека. Прав ли съм? Благодаря. - person ant2009; 15.09.2010
comment
Ако приемем, че имената на вашите кандидати са достатъчно дълги, вместо това опитайте strcpy(candidate_names[i], "Invalid candidate"); - person pmg; 15.09.2010
comment
@robUK: Въпросът е, че съхранението на низовия литерал е в текстовия сегмент на вашия изпълним файл. Не беше разпределено с malloc от купчината. Не можете да го освободите. - person JeremyP; 15.09.2010
comment
@robUK: вие не разпределяте, но getline() го прави. Тази памет трябва да бъде освободена или valgrind се оплаква. - person pmg; 15.09.2010
comment
@JeremyP, сега разбирам. Така низовият литерал беше разпределен в стека. Така че опитът да го освободите ще завърши с грешка „Невалидно безплатно“. Тъй като никога не е бил разпределян при използване на malloc. Благодаря. - person ant2009; 15.09.2010
comment
@robUK: Низовите литерали не се разпределят в стека, но сте прав, че въпросът е, че те не се разпределят с malloc. - person jamesdlin; 15.09.2010
comment
@JeremyP, ако низовите литерали са разпределени в стека, къде се намират те? Низовият литерал е указател, който трябва да има някаква област в паметта. Благодаря. - person ant2009; 16.09.2010
comment
@robUK: може би този SO пост ще помогне ... ( stackoverflow.com/questions/1966920/ ) - person pmg; 16.09.2010
comment
@robUK: Низовите литерали не се разпределят в стека, те се разпределят като част от кода на вашата програма от компилатора. - person JeremyP; 16.09.2010

getline() отделя място за реда, който току-що прочете, извиквайки malloc() за вас зад кулисите. Вие съхранявате тези редови буфери в масива candidate_names, но никога не го освобождавате. Изтичането не е указателят на файла - освобождавате го добре. Това са редовете, които четете от файла, които трябва да бъдат освободени другаде, когато приключите с използването им.

person bdonlan    schedule 15.09.2010

Виждам, че имате два различни макроса NUMBER_OF_CANDIDATES и NAME_SIZE. Прилича на неприятности.

person Jens Gustedt    schedule 15.09.2010
comment
Това може и да е така, но не отговаря на въпроса... Това вероятно би било по-добре като коментар, а не като отговор. - person bdonlan; 15.09.2010
comment
Не е проблемът, но е проблем, особено ако NUMBER_OF_CANDIDATES някога стане по-голям от NAME_SIZE. - person Karl Bielefeldt; 15.09.2010
comment
#define NAME_SIZE 80. За съжаление пропуснах това. - person ant2009; 15.09.2010
comment
@bdonlan: това може просто да е проблемът, тъй като тогава щеше да имаш повредена памет, недефинирано поведение и всичко може да се случи. - person Jens Gustedt; 15.09.2010

Разпределяте памет вътре в getline(). Никога не освобождавате тази памет. Това е, което valgrind ви казва: че имате седем (== NUMBER_OF_CANDIDATES) блока, които не сте освободили.

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

Трябва да направите нещо подобно в края на load_candidates():

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

РЕДАКТИРАНЕ

Във вашата редакция вие освобождавате нулеви указатели. Опитвам:

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]);
        }
    }
}
person Andy Johnson    schedule 15.09.2010

Не мисля, че това е правилно.

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

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

person Novikov    schedule 15.09.2010
comment
getline е стандартна функция на POSIX с прототип `ssize_t getline(char **lineptr, size_t *n, FILE *stream);`. Предаването на указател към-a-size_t е правилно. - person bdonlan; 15.09.2010
comment
опа, по някаква причина си мислех за C++ fstream getline. - person Novikov; 15.09.2010