2009-04-08 2 views
1

Hy all,Выделенная функция для распределения памяти вызывает утечку памяти?

Я считаю, что следующий фрагмент кода создает утечку памяти?

/* External function to dynamically allocate a vector */ 
    template <class T> 
      T *dvector(int n){ 
      T *v; 

      v = (T *)malloc(n*sizeof(T)); 

      return v; 
    } 


    /* Function that calls DVECTOR and, after computation, frees it */ 
    void DiscontinuousGalerkin_Domain::computeFaceInviscidFluxes(){ 
      int e,f,n,p; 
      double *Left_Conserved; 

      Left_Conserved = dvector<double>(NumberOfProperties); 

      //do stuff with Left_Conserved 
      // 

      free(Left_Conserved); 

      return; 
    } 

Я подумал, что, передавая указатель на DVECTOR, было бы выделить его и вернуть правильный адрес, так что бесплатно (Left_Conserved) бы успешно освободить. Однако, похоже, это не так.

Примечание: Я также протестировал с новым/удалить замена таНоса/бесплатно без успеха либо.

У меня есть аналогичный код для выделения 2-мерного массива. Я решил управлять такими векторами/массивами, потому что я их много использую, и мне также хотелось бы понять немного более глубокое управление памятью на C++.

Итак, я бы очень хотел, чтобы внешняя функция выделяла для меня векторы и массивы. Какая уловка здесь, чтобы избежать утечки памяти?

EDIT

Я использую функцию DVECTOR выделить определенные пользователем типы, а также, так что потенциально проблема, я думаю, так как у меня нет Конструкторы называют.

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

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

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

Вот что я хотел бы сделать:

template <class T> 
    T **dmatrix(int m, int n){ 
      T **A; 

      A = (T **)malloc(m*sizeof(T *)); 
      A[0] = (T *)malloc(m*n*sizeof(T)); 

      for(int i=1;i<m;i++){ 
        A[i] = A[i-1]+n; 
      } 

      return A; 
    } 


    void Element::setElement(int Ptot, int Qtot){ 

      double **MassMatrix; 

      MassMatrix = dmatrix<myT>(Ptot,Qtot); 

      FillInTheMatrix(MassMatrix); 

      return; 
    } 

ответ

0

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

  • Распределение с таНос заданных пользователем моих типов был глючный. Однако, когда я изменил его на новый, утечка стала еще хуже, и это связано с тем, что у одного из моих пользовательских типов был конструктор, вызывающий внешнюю функцию без параметров и без правильного управления памятью. Поскольку я назвал эту функцию после конструктора, ошибок в самой обработке не было, а только на выделение памяти. Итак, и правильный конструктор решил одну из основных утечек памяти.

  • Другая утечка была связана с командой buggy memory-deallocation, которую я смог изолировать с помощью Valgrind (и немного терпения, чтобы получить его выход правильно). Итак, вот ошибка (и, пожалуйста, не называйте меня придурок!):

    if (something){ 
         //do stuff 
         return; //and here it is!!! =P 
    } 
    free(); 
    return; 
    

И вот где RAII, как я понял, хотел бы избежать misprogramming так же, как это. Я на самом деле не изменил его на std :: vector или boost :: scoped_array, но мне все еще не ясно, может ли он передать их как параметр другим функциям. Итак, я все еще должен быть осторожен с delete [].

Во всяком случае, утечка памяти исчезла (сейчас ...) = D

0

Я не см. утечку памяти в этом коде.

Если вы пишете программы на C++ - используйте new/delete вместо malloc/free.

Для избежать возможных утечек памяти использовать смарт-указатели или контейнеры STL.

+0

И не забудьте использовать удалить [] для массивов. Это одна из причин, по которой мне нравятся контейнеры STL: меньше простых способов повредить большой. –

0

Нет, если вы не делаете ничего радикального между вызовом вашего шаблона dvector и free, вы не пропускаете какую-либо память. Что говорит вам об утечке памяти?

Могу ли я спросить, почему вы решили создать свои собственные массивы вместо использования контейнеров STL, таких как vector или list? Это наверняка спасло бы вам много троллей.

+0

При выполнении команды команда top показывает выделенную память, растущую бесконечно! Когда я комментирую все операции с памятью в DiscontinuousGalerkin_Domain :: computeFaceInviscidFluxes(), память перестает протекать. Ну, и я вообще не знал вектор. Можно ли создавать с ним 2D-массивы? – Biga

+0

@Biga: вектор похож на 1D массив двойников, вектор <вектор > представляет собой 2D-массив и так далее ... – dirkgently

+0

Результат для этой команды адресуется как ARRAY [i] [j]? А что, если «i» и «j» охватывает разные измерения? – Biga

7

Там нет утечки памяти, но вы должны использовать new/delete [] вместо malloc/free. Тем более, что ваша функция шаблонизирована.

Если вы когда-либо захотите использовать тип, который имеет нетривиальный конструктор, ваша функция на основе malloc будет нарушена, так как она не вызывает никаких конструкторов.

Я бы заменить «dvector» с просто делать это:

void DiscontinuousGalerkin_Domain::computeFaceInviscidFluxes(){ 
     double *Left_Conserved = new double[NumberOfProperties]; 

     //do stuff with Left_Conserved 
     // 

     delete[] Left_Conserved; 
} 

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

еще лучше, использовать смарт-указатели, чтобы полностью избежать утечек памяти:

void DiscontinuousGalerkin_Domain::computeFaceInviscidFluxes(){ 
     boost::scoped_array<double> Left_Conserved(new double[NumberOfProperties]); 

     //do stuff with Left_Conserved 
     // 
} 

Как много умных программистов хотели сказать «лучший код код, который вы не должны написать»

EDIT: Почему вы считаете, что код, который вы опубликовали, хранит память?

EDIT: Я видел ваш комментарий на другой пост говоря

В код команды выполнения верхней показывает распределяемой памяти растет до бесконечности!

Это может быть полностью нормальным (или может быть не) в зависимости от вашего шаблона распределения. Обычно работа кучи заключается в том, что они часто растут, но не часто сокращаются (это способствует последующим распределениям). Полностью симметричные распределения и освобождения должны позволять приложению стабилизироваться при определенном объеме использования.

Например:

while(1) { 
    free(malloc(100)); 
} 

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

Так что мой вопрос к вам. Растет ли он «бесконечно» или просто не сокращается?

EDIT:

Вы спросили, что делать с 2D массива. Лично я бы использовал класс, чтобы обернуть детали. Я бы использовал библиотеку (я считаю, что boost имеет класс n-dimmentional array), или переключение ваших собственных не должно быть слишком сложным.Нечто подобное может быть достаточно:

http://www.codef00.com/code/matrix.h

Использование выглядит следующим образом:

Matrix<int> m(2, 3); 
m[1][2] = 10; 

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

EDIT: другой вопрос. На какой платформе вы работаете? Если это * nix, то я бы рекомендовал valgrind помочь точно определить утечку памяти. Поскольку код, который вы предоставили, явно не проблема.

Я не знаю ни одного, но я уверен, что окна также имеют инструменты профилирования памяти.

EDIT: для матрицы, если вы настаиваете на использование простых старых массивов, почему бы не просто выделить его как единый непрерывный блок и сделать простую математику на индексации, как это:

T *const p = new T[width * height]; 

затем для получения доступа к элемент, просто сделать это:

p[y * width + x] = whatever; 

таким образом вы делаете delete[] на указатель, является ли это массив 1D или 2D.

+0

У меня также есть внешняя функция, выделяющая 2-мерный массив, поэтому я не просто написал его, как вы предлагали. Я пытался избежать копирования/вставки всех строк для динамического выделения двухмерного массива. Есть ли разумный способ сделать это? – Biga

+0

Он растет бесконечно, и через некоторое время ПК зависает! (kinda freaking!) – Biga

+0

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

1

Нет видимой утечки памяти, однако существует высокая вероятность утечки памяти с таким кодом. Старайтесь всегда обтекать ресурсы в объекте (RAII). станд :: вектор делает именно то, что вы хотите:

void DiscontinuousGalerkin_Domain::computeFaceInviscidFluxes(){ 
     int e,f,n,p; 
     std::vector<double> Left_Conserved(NumOfProperties);//create vector with "NumOfProperties" initial entries 

     //do stuff with Left_Conserved 
     //exactly same usage ! 
     for (int i = 0; i < NumOfProperties; i++){//loop should be "for (int i = 0; i < Left_Conserved.size();i++)" .size() == NumOfProperties ! (if you didn't add or remove any elements since construction 
      Left_Conserved[i] = e*f + n*p*i;//made up operation 
     } 
     Left_Conserved.push_back(1.0);//vector automatically grows..no need to manually realloc 
     assert(Left_Conserved.size() == NumOfProperties + 1);//yay - vector knows it's size 
     //you don't have to care about the memory, the Left_Conserved OBJECT will clean it up (in the destructor which is automatically called when scope is left) 
     return; 
} 

EDIT: добавлено несколько примеров операций. Вы действительно должны прочитать о STL-контейнерах, они того стоят!
EDIT 2: для 2d вы можете использовать:

std::vector<std::vector<double> > 

, как кто-то предложил в комментариях. но использование с 2d немного сложнее. Вы должны сначала изучить 1-й случай, чтобы узнать, что происходит (увеличение векторов и т. Д.)

+0

Что касается двухмерного массива, например Properties [NumOfProperties] [NumOfProperties]? Есть ли также ЗППП? – Biga

+0

Is Left_Conserved указатель, если выделено вектором? – Biga

0

Что произойдет, если вы передадите отрицательное значение для n в dvector?

Возможно, вам следует рассмотреть вопрос об изменении вашей подписи функции взять без знака типа в качестве аргумента:

template< typename T > 
T * dvector(std::size_t n); 

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

template< typename T > 
void dvector_free(T * p) { free(p); } 

Как другие предположили, выполнение этого как класса RAII было бы более надежным. И, наконец, как и другие, также было предложено, для этого есть множество существующих, проверенных временем библиотек, поэтому вам может не понадобиться вообще сворачивать свои собственные.