Это хороший способ освободить память в C?

Ответ 1

Нам действительно нужно назначить указатель на временный? Имеет ли это помощь с точки зрения concurrency и разделяемой памяти?

Ему нечего делать concurrency или разделяемой памяти. Это бессмысленно.

Действительно ли это хорошая идея, чтобы весь блок был равен 0, чтобы заставить программа для сбоя или, по крайней мере, для вывода результатов со значительными расхождение?

Нет. Совсем нет.

Решение, предложенное вашим коллегой, ужасно. Вот почему:

• Установка целых блоков на 0 тоже ничего не дает. Поскольку кто-то случайно использует блок free() 'ed, они не знают, что на основе значений в блоке. Это возвращает тип блока calloc(). Таким образом, невозможно узнать, была ли она недавно выделенной памятью (calloc() или malloc()+memset()) или той, которая была ранее() изменена вашим кодом ранее. Если что-то лишнее работает для вашей программы, чтобы обнулить каждый блок памяти, который является бесплатным().

• free(NULL); четко определен и не является оператором, поэтому условие if в if(ptr) {free(ptr);} ничего не достигает.

• Так как free(NULL); не работает, установка указателя на NULL фактически скроет эту ошибку, потому что если какая-то функция фактически вызывает free() на уже свободном() 'указателе, то они не знал бы этого.

• большинство пользовательских функций будут иметь NULL-проверку в начале и не могут рассматривать передачу NULL в качестве условия ошибки:

void do_some_work(void *ptr) {
    if (!ptr) {
        return; 
    }

   /*Do something with ptr here */
}

Таким образом, все эти дополнительные проверки и zero'ing out дают поддельное чувство "надежности", в то время как это ничего не улучшило. Он просто заменил одну проблему другой дополнительной стоимостью производительности и раздуванием кода.

Таким образом, просто вызов free(ptr); без какой-либо функции-обертки является простым и надежным (большинство malloc() реализаций немедленно сработают сразу же после двойного освобождения, что хорошо).

Нет простого способа "случайно" вызвать free() дважды или более. Обязанность программиста - отслеживать всю выделенную память и free(). Если кому-то это трудно справиться, тогда C, вероятно, не подходит для них.

Ответ 2

То, что предлагает ваш коллега, сделает код "более безопасным", если функция вызывается дважды (см. комментарий sleske... как "безопаснее", возможно, не означает одно и то же для всех...; -).

С вашим кодом это скорее всего сбой:

Foo* foo = malloc( sizeof(Foo) );
DestroyFoo(foo);
DestroyFoo(foo); // will call free on memory already freed

С вашей версией кода коллеги это не сработает:

Foo* foo = malloc( sizeof(Foo) );
DestroyFoo(&foo);
DestroyFoo(&foo); // will have no effect

Теперь для этого конкретного сценария достаточно сделать tmpFoo = 0; (внутри DestroyFoo). memset(tmpFoo, 0, sizeof(Foo)); предотвратит сбой, если Foo имеет дополнительные атрибуты, которые могут быть ошибочно доступны после освобождения памяти.

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

Ответ 3

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

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

Foo* foo = malloc( sizeof(Foo) );
DestroyFoo(foo);
foo = NULL;

Более того, я не знаю, почему люди проверяют, является ли указатель NULL перед вызовом free(). Это не нужно, так как free() выполнит эту работу для вас.

Установка памяти на 0 (или что-то еще) является лишь в некоторых случаях хорошей практикой, поскольку free() не очищает память. Он просто отметит, что область памяти свободна, чтобы ее можно было повторно использовать. Если вы хотите очистить память, чтобы никто не смог ее прочитать, вам нужно ее очистить вручную. Но это довольно тяжелая операция, и почему это не должно использоваться для освобождения всей памяти. В большинстве случаев освобождения без очистки достаточно, и вам не нужно жертвовать производительностью, чтобы делать ненужную работу.

Ответ 4

void destroyFoo(Foo** foo)
{
    if (!(*foo)) return;
    Foo *tmpFoo = *foo;
    *foo = NULL;
    memset(tmpFoo, 0, sizeof(Foo));
    free(tmpFoo);
}

Ваш код коллеги плохо, потому что

• он сработает, если foo - NULL
• Нет смысла создавать дополнительную переменную
• нет смысла устанавливать значения в нули
• освобождение структуры напрямую не работает, если оно содержит вещи, которые должны быть освобождены

Я думаю, что ваш коллега мог иметь в виду этот случай использования

Foo* a = NULL;
Foo* b = createFoo();

destroyFoo(NULL);
destroyFoo(&a);
destroyFoo(&b);

В этом случае это должно быть так. попробуйте здесь

void destroyFoo(Foo** foo)
{
    if (!foo || !(*foo)) return;
    free(*foo);
    *foo = NULL;
}

Сначала нам нужно взглянуть на foo, предположим, что это выглядит как

struct Foo
{
    // variables
    int number;
    char character;

    // array of float
    int arrSize;
    float* arr;

    // pointer to another instance
    Foo* myTwin;
};

Теперь, чтобы определить, как он должен быть уничтожен, сначала определите, как он должен быть создан

Foo* createFoo (int arrSize, Foo* twin)
{
    Foo* t = (Foo*) malloc(sizeof(Foo));

    // initialize with default values
    t->number = 1;
    t->character = '1';

    // initialize the array
    t->arrSize = (arrSize>0?arrSize:10);
    t->arr = (float*) malloc(sizeof(float) * t->arrSize);

    // a Foo is a twin with only one other Foo
    t->myTwin = twin;
    if(twin) twin->myTwin = t;

    return t;
}

Теперь мы можем написать функцию destroy, противоположенную функции create

Foo* destroyFoo (Foo* foo)
{
    if (foo)
    {
        // we allocated the array, so we have to free it
        free(t->arr);

        // to avoid broken pointer, we need to nullify the twin pointer
        if(t->myTwin) t->myTwin->myTwin = NULL;
    }

    free(foo);

    return NULL;
}

Тест попробуйте здесь

int main ()
{
    Foo* a = createFoo (2, NULL);
    Foo* b = createFoo (4, a);

    a = destroyFoo(a);
    b = destroyFoo(b);

    printf("success");
    return 0;
}

Ответ 5

К сожалению, эта идея просто не работает.

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

Предположим, что этот код:

Foo *ptr_1 = (FOO*) malloc(sizeof(Foo));
Foo *ptr_2 = ptr_1;
free (ptr_1);
free (ptr_2); /* This is a bug */

Вместо этого предлагается написать:

Foo *ptr_1 = (FOO*) malloc(sizeof(Foo));
Foo *ptr_2 = ptr_1;
DestroyFoo (&ptr_1);
DestroyFoo (&ptr_2); /* This is still a bug */

Проблема в том, что второй вызов DestroyFoo() прежнему будет сбой, потому что ptr_2 не сбрасывается в NULL и все еще указывает на освобожденную память.