2011-01-10 3 views
-3

Уважаемые участники, У меня путают, освобождаю ли я память в правильном месте или нет. специально * sResult?есть утечка памяти, я освобождаю память правильно

int ReadToSerialPort(char *psResponse, int iMax) 
{ 

    size_t iIn; 

    if (fd < 1) 
    { 
     printf("port is not open\n"); 
     return -1; 
    }  

    iIn = read(fd, psResponse, iMax-1);  
    if (iIn < 0)  
    { 
     if (errno == EAGAIN) 
     { 
      printf("The errror in READ"); 
      return 0; // assume that command generated no response  
     } 
     else 
      printf("read error %d %s\n", errno, strerror(errno)); 

    } 
    else 
psResponse[(int)iIn<(int)iMax?iIn:iMax] = '\n'; 

    return iIn; 

} // end ReadAdrPort 
int MultiQuery() 
{ 
    // check database connectivity 

// code to check DB 

    while (1) 
    { //while start 

     // char *sResult = NULL; 
      char *sResult = (char *)malloc(4096); 

      // Reading from H/W 
      if ((ReadToSerialPort(sResult,4096)) > 0)  
      { 

       // code to trim read line and put into db.... 



      printf(" before free is %s\n", sResult); 

      free(sResult);   
      sResult = NULL;  
     } // end ifReadToSerialPort >0  

     //*sResult = NULL;  


    } // while(1) ends;  

    fclose(errorlog); 
    fclose(errorlog2); 
    mysql_close(&mysql); 
    return 0; 
} 
+9

Удаленный тег C, поскольку здесь имеется значительное количество C++-специфического кода. Я также отказался от этого, потому что вы просто бросили на нас сотни строк кода, не делая для этого никакой работы. Вы думаете, что он просачивается? Проходили ли вы и проверяли все ваши ассигнования против ваших освобождений? – Puppy

+3

Я не проголосовал за вопрос, но ваш код отформатирован ужасно. – Alain

+1

Значит, вы все это сами написали, используя части библиотеки C++ и mySQL API, и понятия не имеете, нужно ли что-либо освобождать? Лично я не думаю, что вы написали это. –

ответ

7

Вы смотрели в инструменты для обнаружения утечек памяти, таких как Valgrind? (Windows substitutes)

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

Это не полная программа, поэтому мы не сможем пройти ее для вас. Некоторые советы:

  • Разбивайте процедуры и выполняйте отдельные тестовые исполняемые файлы, чтобы вы могли гарантировать, что бит вашего кода не содержит утечек памяти. Это похоже на тестовую разработку - вы пишете тест для функции, чтобы убедиться, что он работает и правильно проверяет вход, и вы также проверяете его на утечки памяти.
  • Разработка в библиотеке, как мода. Это должно быть второй натурой, поскольку он оптимизирует повторное использование кода (не повторяйте себя, если только повторение себя не является выгодным) и помогает в этом.

Редактировать: Я расскажу об этом. Чтобы дать Вам идею о том, когда происходит утечка памяти, подумайте:

#include <iostream> 

using namespace std; 

int main(int argc, char** argv) 
{ 
    int* arr = new int(5); 
    // delete arr; // <-- uncomment this to fix memalloc bug. 
    return 0; 
} 

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

Valgrind бы вам сказать:

HEAP SUMMARY: 
==23008==  in use at exit: 4 bytes in 1 blocks 
==23008== total heap usage: 1 allocs, 0 frees, 4 bytes allocated 
==23008== 
==23008== LEAK SUMMARY: 
==23008== definitely lost: 4 bytes in 1 blocks 
==23008== indirectly lost: 0 bytes in 0 blocks 
==23008==  possibly lost: 0 bytes in 0 blocks 
==23008== still reachable: 0 bytes in 0 blocks 
==23008==   suppressed: 0 bytes in 0 blocks 

Почему? Поскольку у вас нет delete d той памяти, которую вы выделили new. Это, грубо говоря, столь же сложно, как и получается.

Я говорю сложно, потому что программы становятся большими кодовыми базами довольно быстро, и утечки памяти внезапно усложняются. Кто выделил память? Кто его освободил? Была ли выделена память как часть библиотеки? Что произойдет, когда вы начнете использовать массивы? Вы можете вызвать косвенную потерю памяти, освободив внешний уровень массива, но не внутренний. См. indirect memory loss.

Это быстро становится очень сложным, особенно когда функции начинают выделять память, и вы начинаете использовать их в другом месте, и вы начинаете полагаться на программно определенные размеры памяти, такие как длины строк на основе пользовательского ввода. Ошибка maxlen+1 является распространенной ошибкой и может создавать большие утечки в больших 2D-массивах данных.

Edit 2 на основе вашего нового вопроса:

Во-первых, если вы используете C++, я настоятельно рекомендую забыть о malloc. Используйте new и delete, потому что они понимают полиморфизм и объекты, тогда как malloc - нет. Возможно, ваши конструкторы не будут правильно вызваны. И если вам нужны строки, используйте тип string. Если вам нужны массивы, обычно будет делать vector.

Во-вторых, вы освобождаете память только в том случае, если операция чтения работает правильно. Зачем? Вы все еще выделили память, поэтому вам все равно нужно ее освободить, независимо от того, работала ли операция чтения.Вы также должны проверить, действительно ли выделена память, иначе вы получите ошибку сегментации, вызванную доступом к памяти, к которой вы не должны получить доступ.

Редактировать три:

Ok, на основе ваших комментариев, рассмотрит этот код, который является то, что я думаю, что вы предлагаете:

char* buffer = (char*)malloc(4096*sizeof(char)); 

if (ReadToSerialPort(buffer, ...) > 0) // you are proposing to free in this func call 
{ 
    // do stuff A 

} 

// do stuff B 

// free(buffer); // <-- you should free buffer here 

Что произойдет, если вы свободны в вызове функции? Ну, на самом деле это нормально, кроме того, что в этом коде есть еще переменная, которую вы можете случайно использовать. Если вы это сделаете, вы получите доступ к памяти, которую вы явно выпустили, что, вероятно, приведет к сбою вашей программы.

Я предполагаю, что на каком-то этапе вы захотите использовать этот буфер либо в частях A или B. В этом случае вам нужно, чтобы память еще была выделена.

Что касается освобождения внутри оператора if, это гарантированная утечка памяти, если функция чтения не работает, потому что эта память никогда не освобождается.

Вот как вы должны это сделать:

// 
// I maintain you should use a string here, or 
// if you insist on char*, use new. 
// and delete. 
// 
char* buffer = (char*)malloc(4096*sizeof(char)); 

if (buffer == NULL) 
{ 
    // os refused the alloc. 
    cout << "Out of memory\n" << endl; 
    return 1; 
} 

if (ReadToSerialPort(buffer, ...) > 0) // this call frees as well? 
{ 
    // do stuff A using buffer 

} 

// do stuff B using buffer 

free(buffer); // <-- you should free buffer here 

Редактировать 4: Просто для уточнения:

Не следует смешивать таНос/бесплатно и новых/удалить. Если вы malloc, освободите эту память, если вы новичок, удалите эту память. Если вы взаимодействуете с кодом C, если для этого не требуется массив типа int, вы можете использовать тип string, и нет необходимости в динамическом распределении. Если вы не переходите к функции C, у вас нет контроля, вы должны, вероятно, рассмотреть vector над массивом. Только если вы переходите к функции C, у вас нет контроля над тем, если вам нужно войти в мир Malloc и бесплатных методов C.

+0

Спасибо Ninefingers, Но я освобождаю память. Меня беспокоит только то, что я высвобождаюсь в нужном месте или нет. А также вместо бесплатного (sResult), если я дам * sResult = NULL чуть выше, пока (1) заканчивается, какая разница будет в моей проге. – samprat

+0

Я добавил освобождение кода внутри ReadToSerialPort> 0, потому что я думал, что выделение памяти выполняется во время выполнения, и если ReadToSerialPort что-то читает, то лучше освободить его. А также, если я добавлю бесплатный (sResult) до того, как (1) закончится, он освободит память, которую я havnt выделял иногда, как в случае, если ReadToSerialPort <0 – samprat

+0

Не очень хорошая идея, тем более, что вы пытались написать NULL этому адрес памяти в более поздней точке кода! Изменить - ах, который был прокомментирован. Но посмотрите, ясно ли, что происходит с этой памятью от вашего вызова ReadToSerialPort при чтении кода? –

1

Быстрое использование Ctrl + F (потому что вы выложили слишком много нерелевантного кода), показывает, что единственная память, которую вы выделяете, предназначена для * sResult, и до того, как вы освободите эту память, нет предложений выхода, поэтому Да. После сглаживания этого кода я стесняюсь использовать фразу «вы делаете это правильно», но вы это делаете.

+0

Спасибо Ален, так ли это, что я освобождаю память в правильном месте ?. Но что, если ReadTOSerialPort <0, но выше, я выделяю SResult = (char *) malloc (4096). Итак, есть ли какой-либо недостаток, если ReadToSerialPort <0, поскольку я освобождаю память только тогда, когда она больше 0. – samprat

+0

Это верно только в том случае, если функции, которые он называет между 'malloc' и' free', не генерируют никаких исключений, так как у него нет обработки исключений в этой стене кода, который он опубликовал. Это не проблема, если ни одна из функций, которые он вызывает, может «бросить». –

+0

samprat, у вас было два вызова бесплатного оператора - один был вне оператора if. Теперь, когда вы обрезали количество отправленных кодов, я не вижу, где это. – Alain

Смежные вопросы