Нужно действительно установить указатели на 'ПУСТОЙ УКАЗАТЕЛЬ' после освобождения их?

Если вы не установите указатель на NULL там - это не так уж и малая вероятность, что ваше приложение продолжит работать в неопределенном состоянии и выйдет из строя позже в совершенно несвязанной точке. Тогда вы потратите много времени на отладку несуществующей ошибки, прежде чем обнаружите, что это повреждение памяти, которое было ранее.

I ' d установите указатель в NULL, потому что шансы, что вы попадете в правильное место ошибки раньше, выше, чем если бы вы не установили его в NULL. Логическая ошибка освобождения памяти во второй раз все еще требует размышлений, и ошибка, заключающаяся в том, что ваше приложение НЕ аварийно завершает работу при доступе к нулевому указателю с достаточно большим смещением, на мой взгляд, является полностью теоретической, хотя и не невозможной.

Заключение: I Пойдем для установки указателя в NULL.

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

Однажды мне пришлось работать над действительно глючная программа еще кто-то написал. Мои инстинкты подсказали мне, что многие ошибки были связаны с небрежными попытками продолжать использовать указатели после освобождения памяти; Я изменил код, чтобы установить указатели в NULL после освобождения памяти, и bam начали поступать исключения нулевого указателя. После того, как я исправил все исключения нулевого указателя, код внезапно стал намного более стабильным.

В моем собственном коде я вызываю только свою собственную функцию, которая является оболочкой вокруг free (). Он принимает указатель на указатель и обнуляет указатель после освобождения памяти. И перед тем, как вызвать free, он вызывает Assert (p! = NULL); , поэтому он по-прежнему отлавливает попытки дважды освободить тот же указатель.

Мой код выполняет и другие вещи, например (в Только сборка DEBUG) заполняет память очевидным значением сразу после ее выделения, делая то же самое прямо перед вызовом free () в случае, если есть копия указателя и т. Д. Подробности здесь.

] РЕДАКТИРОВАТЬ: для запроса, вот пример кода.

void
FreeAnything(void **pp)
{
    void *p;

    AssertWithMessage(pp != NULL, "need pointer-to-pointer, got null value");
    if (!pp)
        return;

    p = *pp;
    AssertWithMessage(p != NULL, "attempt to free a null pointer");
    if (!p)
        return;

    free(p);
    *pp = NULL;
}


// FOO is a typedef for a struct type
void
FreeInstanceOfFoo(FOO **pp)
{
    FOO *p;

    AssertWithMessage(pp != NULL, "need pointer-to-pointer, got null value");
    if (!pp)
        return;

    p = *pp;
    AssertWithMessage(p != NULL, "attempt to free a null FOO pointer");
    if (!p)
        return;

    AssertWithMessage(p->signature == FOO_SIG, "bad signature... is this really a FOO instance?");

    // free resources held by FOO instance
    if (p->storage_buffer)
        FreeAnything(&p->storage_buffer);
    if (p->other_resource)
        FreeAnything(&p->other_resource);

    // free FOO instance itself
    free(p);
    *pp = NULL;
}

Комментарии:

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

Мои настоящие имена функций скорее больше лаконично, но я попытался выбрать для этого примера самодокументирующиеся имена. Кроме того, в моем реальном коде у меня есть код, предназначенный только для отладки, который заполняет буферы значением 0xDC перед вызовом free () , так что если у меня есть дополнительный указатель на ту же самую память ( тот, который не обнуляется) становится действительно очевидным, что данные, на которые он указывает, являются поддельными. У меня есть макрос DEBUG_ONLY () , который ничего не компилирует в не отладочной сборке; и макрос FILL () , который выполняет sizeof () в структуре. Эти два работают одинаково хорошо: sizeof (FOO) или sizeof (* pfoo) . Итак, вот макрос FILL () :

#define FILL(p, b) \
    (memset((p), b, sizeof(*(p)))

Вот пример использования FILL () для ввода значений 0xDC перед вызовом:

if (p->storage_buffer)
{
    DEBUG_ONLY(FILL(pfoo->storage_buffer, 0xDC);)
    FreeAnything(&p->storage_buffer);
}

Пример использования этого:

PFOO pfoo = ConstructNewInstanceOfFoo(arg0, arg1, arg2);
DoSomethingWithFooInstance(pfoo);
FreeInstanceOfFoo(&pfoo);
assert(pfoo == NULL); // FreeInstanceOfFoo() nulled the pointer so this never fires

Ответ зависит от (1) размера проекта, (2) ожидаемого времени жизни вашего кода, (3) размера команды. В небольшом проекте с коротким жизненным циклом вы можете не устанавливать указатели на NULL и просто выполнять отладку.

В большом, долгоживущем проекте есть веские причины для установки указателей на NULL: (1) Защитное программирование - это всегда хорошо. Ваш код может быть в порядке, но новичок по соседству может все еще бороться с указателями (2) Я лично считаю, что все переменные всегда должны содержать только допустимые значения. После удаления / освобождения указатель больше не является допустимым значением, поэтому его необходимо удалить из этой переменной. Замена его на NULL (единственное всегда действительное значение указателя) - хороший шаг. (3) Код никогда не умирает. Его всегда используют повторно, и часто так, как вы не представляли, когда писали его. Ваш сегмент кода может оказаться скомпилированным в контексте C ++ и, вероятно, будет перемещен в деструктор или метод, который вызывается деструктором. Взаимодействие виртуальных методов и объектов, находящихся в процессе разрушения, является тонкой ловушкой даже для очень опытных программистов. (4) If your code ends up being used in a multi-threaded context, some other thread might read that variable and try to access it. Such contexts often arise when legacy code is wrapped and reused in a web server. So an even better way of freeing memory (from a paranoid standpoint) is to (1) copy the pointer to a local variable, (2) set the original variable to NULL, (3) delete/free the local variable.

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

void release() {
    assert(m_pt!=NULL);
    T* pt = m_pt;
    m_pt = NULL;
    free(pt);
}

T* operator->() {
    assert(m_pt!=NULL);
    return m_pt;
}

В качестве альтернативы в C вы могли бы по крайней мере предоставить два макроса для одного и того же эффект:

#define SAFE_FREE(pt) \
    assert(pt!=NULL); \
    free(pt); \
    pt = NULL;

#define SAFE_PTR(pt) assert(pt!=NULL); pt

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

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

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

These problems are most often only symptoms for a much deeper problem. This can occur for all resources that require aquisition and a later release, e.g. memory, files, databases, network connections, etc. The core problem is that you've lost track of the resource allocations by a missing code structure, throwing random mallocs and frees all over the code base.

Organize the code around DRY - Don't Repeat Yourself. Keep related things together. Do one thing only, and do it good. The "module" that allocates a resource is responsible for releasing it and has to provide a function for doing so that keeps care for the pointers, too. For any specific resource, you then have exactly one place where it is allocated and one place where it is released, both close together.

Say you want to split a string into substrings. Directly using malloc(), your function has to care for everything: Analysing the string, allocate the right amount of memory, copy the substrings there, and and and. Make the function complicated enough, and it is not the question if you will lose track of the resources, but when.

Your first module takes care for the actual memory allocation:

    void *MemoryAlloc (size_t size)
    void  MemoryFree (void *ptr)

There's your only place in your whole codebase where malloc() and free() are called.

Then we need to allocate strings:

    StringAlloc (char **str, size_t len)
    StringFree (char **str)

They take care that len+1 is needed and that the pointer is set to NULL when freed. Provide another function to copy a substring:

    StringCopyPart (char **dst, const char *src, size_t index, size_t len)

It will take care if index and len are inside the src string and modify it when needed. It will call StringAlloc for dst, and it will care that dst is correctly terminated.

Now you can write your split function. You don't have to care for the low level details anymore, just analyse the string and get the substrings out of it. Most of the logic is now in the module where it belongs, instead of mixed together in one large monstrosity.

Of course this solution has its own problems. It provides abstraction layers, and each layer, while solving other problems, comes with its own set of them.

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

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

Нет гарантии, что программа выйдет из строя при доступе к NULL-указателю.

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

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

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

В третьем случае вы устанавливаете указатель в NULL. Причина не в том, что вы его освобождаете, а потому, что все, что есть, необязательно, поэтому, конечно, NULL - это специальное значение, означающее «У меня его нет».

В первых двух случаях установка указателя на NULL кажется мне занятой работой без особой цели:

int doSomework() {
    char *working_space = malloc(400*1000);
    // lots of work
    free(working_space);
    working_space = NULL; // wtf? In case someone has a reference to my stack?
    return result;
}

int doSomework2() {
    char * const working_space = malloc(400*1000);
    // lots of work
    free(working_space);
    working_space = NULL; // doesn't even compile, bad luck
    return result;
}

void freeTree(node_type *node) {
    for (int i = 0; i < node->numchildren; ++i) {
        freeTree(node->children[i]);
        node->children[i] = NULL; // stop wasting my time with this rubbish
    }
    free(node->children);
    node->children = NULL; // who even still has a pointer to node?

    // Should we do node->numchildren = 0 too, to keep
    // our non-existent struct in a consistent state?
    // After all, numchildren could be big enough
    // to make NULL[numchildren-1] dereferencable,
    // in which case we won't get our vital crash.

    // But if we do set numchildren = 0, then we won't
    // catch people iterating over our children after we're freed,
    // because they won't ever dereference children.

    // Apparently we're doomed. Maybe we should just not use
    // objects after they're freed? Seems extreme!
    free(node);
}

int replace(type **thing, size_t size) {
    type *newthing = copyAndExpand(*thing, size);
    if (newthing == NULL) return -1;
    free(*thing);
    *thing = NULL; // seriously? Always NULL after freeing?
    *thing = newthing;
    return 0;
}

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

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

if (thing->cached != NULL) {
    assert(thing->cached != NULL);
    free(thing->cached);
    thing->cached = NULL;
}
free(thing);

Этот код сообщает вам, что вы зашли слишком далеко. Должно быть:

free(thing->cached);
free(thing);

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

free(thing->cached);
thing->cached = (void*)(0xFEFEFEFE);

Если вы можете Если в вашей системе нет такой константы, вы можете выделить нечитаемую и / или не записываемую страницу и использовать ее адрес.