Как освободить () malloc () правильно структурированы? - PullRequest
7 голосов
/ 02 февраля 2010

У меня есть структура malloc () 'd, и после их использования я хочу освободить ее (), но моя программа зависает здесь. Может кто-нибудь сказать мне, что я делаю не так?

Вот мой код:

struct data  
{  
char *filename;  
char *size;  
};   
 //primarypcs is a long type variable
struct data *primary = (struct data *)malloc( primarypcs * sizeof( struct data ) );  
memset( primary, 0, sizeof(struct data *) * primarypcs );  
...
...
...
for ( i = 0; i < primarypcs; i++ )  
{
   free( primary[i].filename );  //<----my program freezes here
   free( primary[i].size );      //<----or here
}
free( primary );  

Заранее спасибо!

Kampi

EDIT:

Как я могу правильно использовать память malloc для имени файла и размера?

РЕДАКТИРОВАТЬ2:

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

Ответы [ 8 ]

7 голосов
/ 02 февраля 2010

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

memset( primary, 0, sizeof(struct data *) * primarypcs );   

не делает то, что, как вы думаете, делает. Это не обнуление всего массива из-за ошибки типа в sizeof. Скорее всего, это должно было быть

memset( primary, 0, sizeof(struct data) * primarypcs );   

Примечание нет * под sizeof. Из-за этой ошибки большинство указателей в вашем массиве содержат мусор в качестве своих начальных значений. Если вы не установите для них что-то значимое в пропущенном коде, ваши вызовы free получат аргументы мусора и завершатся неудачей.

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

struct data *primary = (struct data *) malloc( primarypcs * sizeof *primary );   
memset( primary, 0, primarypcs * sizeof *primary );   

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

data *primary = new data[primarypcs]();

Конечно, в этом случае вам придется освободить память, используя соответствующую функциональность C ++ вместо free.

3 голосов
/ 02 февраля 2010

Как расположены строки в структуре? Если они статически присвоены константам, то не освобождайте их таким образом, и все, что нужно, это free (primary); Освобождение чего-то, что не было malloc'd, вызовет у менеджера кучи приступ сердца.

Если строковые указатели установлены с помощью malloc () или calloc (), то это правильный путь.

2 голосов
/ 02 февраля 2010

Если вы делаете это в C ++, вы (почти наверняка) не должны использовать что-то вроде:

data *primary = new data[primarypcs]();

Вместо этого вы должны использовать что-то вроде:

struct data {
    std::string filename;
    std::string size;
};

std::vector<data> primary(primarypcs);

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

Использование массива new (например, new x[y]) в C ++ - это то, без чего вам лучше. Когда-то (15 лет назад или около того) он был почти единственным доступным инструментом, поэтому его (неохотное) использование было почти неизбежным - но этот день давно прошел, и это было в последние 10 лет, так как действительно был хороший причина использовать его.

Поскольку неизбежно есть комментарий о «кроме реализации чего-то вроде вектора», я укажу, что нет, даже когда вы реализуете вектор, вы не используете массив new - вы (косвенно, через распределитель) ) используйте ::operator new для выделения необработанной памяти, размещения новых для создания объектов в этой памяти и явных вызовов dtor для уничтожения объектов.

1 голос
/ 02 февраля 2010

Как уже говорили другие, во фрагменте, который вы показали, есть две явно неправильные вещи:

  1. Вы не выделяете память для filename и size членов только что выделенных структур,
  2. Ваш memset() звонок использует неправильный размер.

Ваш memset() вызов может быть упрощен и исправлен с помощью:

memset(primary, 0, primarypcs * sizeof *primary);

Есть еще одна тонкая проблема с вашим кодом: стандарт C не гарантирует, что все биты-ноль являются константой нулевого указателя (т. Е. NULL), поэтому memset () не является правильным способом установки указателя на NULL. Портативный способ сделать то, что вы хотите сделать, это:

size_t i;
for (i=0; i < primarypcs; ++i) {
    primary[i].filename = NULL;
    primary[i].size = NULL;
}

Чтобы выделить память для filename и size, это зависит от того, что вы хотите. Допустим, вы определили, что filename нужно n байтов, а size нужно m. Затем ваш цикл меняется на что-то вроде этого:

size_t i;
for (i=0; i < primarypcs; ++i) {
    size_t n, m;
    /* get the values of n and m */
    primary[i].filename = malloc(n * sizeof *primary[i].filename);
    primary[i].size = malloc(m * sizeof *primary[i].size);
}

Вы можете опустить умножение на sizeof *primary[i].filename и sizeof *primary[i].size из приведенного выше, если хотите: C гарантирует, что sizeof(char) равно 1. Я написал выше для полноты и для случая, когда filename и size изменить типы.

Также обратите внимание, что если filename - строка длиной k, то вам нужно (k+1) байт для нее из-за окончания 0 (поэтому n == k+1 выше).

Если бы я угадал, вы хотите, чтобы size сохранил длину соответствующего filename? Если это так, size должен быть не char *, а size_t. Но так как я не знаю, как вы планируете использовать filename и size, я не уверен.

Обязательно проверьте возвращаемое значение malloc(). Возвращает NULL для сбоя. Я упустил проверку из приведенного выше кода для простоты.

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

0 голосов
/ 02 февраля 2010

Вы не можете установить memset для всего массива, что приводит к освобождению указателя мусорной памяти.Используйте calloc вместо malloc / memset, чтобы избежать этой ошибки:

struct data *primary = calloc(primarypcs, sizeof(struct data));

Это одновременно выделяет и очищает память.Если вы хотите инициализировать также все записи struct data:

for (i = 0; i < primarypcs; ++i) {
    primary[i].filename = malloc(...);
    primary[i].size = malloc(...);
}

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

0 голосов
/ 02 февраля 2010

Это исправляет вашу проблему в memset, которая вызвала у вас все виды проблем.

memset( primary, 0, sizeof(struct data) * primarypcs );  

Короче говоря, вы оставили неинициализированную память в конце вашей «первичной» структуры.

0 голосов
/ 02 февраля 2010

Это потому, что вы явно не выделяется память для filename и size. Так что пытается сделать free( primary[i].filename ); или free( primary[i].size ); будет ссылаться Неопределенное поведение .

Just free(primary) достаточно.

EDIT

Этот вопрос был помечен C ++. Так как C ++ является использование new вместо malloc для определенных пользователем типов.

Для различия между new и malloc посмотреть на это.

В C ++ , вам просто нужно написать

 data *primary = new data[primarypcs](); //() for value initialization
0 голосов
/ 02 февраля 2010

Замена цикла for в нижней части кода на free (primary); должна работать.

...