Как стать автором
Обновить

Следует ли проверять указатель на NULL перед вызовом функции free?

Уровень сложностиСредний
Время на прочтение8 мин
Количество просмотров12K
Всего голосов 66: ↑65 и ↓1+84
Комментарии35

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

Очень спорная тема. if (ptr) free(ptr) всегда исполнится намного быстрее если ptr == NULL. Потому что вызов подпрограммы – дорогая операция, а проверка на 0 очень дешевая.

Конечно, если мы знаем что ptr == 0 очень редко, то да, лучше каждый раз вызывать free() – код будет чище, а скорость исполнения практически не пострадает.

Но если наоборот, ptr == 0 почти всегда и только время от времени <> 0, то выигрыш в скорости исполнения может быть очень большой.

Вообще не спорная. В статье это описано, но ещё раз:

  • Нет смысла говорить о подобных оптимизациях, если используются такие медленные операции как malloc/free. Если нужна скорость, нужно думать как сократить количество этих операций. Или, например, выделять небольшие буферы создавать на стеке. Или ещё что-то придумать, но точно не подобный if писать.

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

если используются такие медленные операции как malloc/free

Вы вообще, мой пост прочитали? Если предполагается, что в 99% ptr будет NULL, то тем более, если free медленная, лучше проверить не вызывая ее.

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

Очень сильно сомневаюсь, что они такое могут сделать. Если считаете наоборот – доказательства в студию.

Если предполагается, что в 99% ptr будет NULL, то тем более, если free медленная, лучше проверить не вызывая ее.

Это какая-то особая программа, где 99% процентов указателей это NULL :)

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

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

Это просто пример. Можно что-то ещё придумать. Смысл в том, что разумно оптимизировать такие ситуации с верхнего уровня, а не снизу по мелочи. Не там ищется решение задачи оптимизации.

Да блин, представьте, что вы пишете деструктор дерева. Хоть двоичного, хоть В-звездатого. В нём всегда нулевых указателей будет больше, чем ненулевых (примерно в В раз в лучшем случае)

Предполагаю, что она будет медленная только в том случае, если ptr все-таки не NULL.

Я не об этом. Сам факт вызова замедляет выполнение. Хоть даже если там ret сразу стоит. А там не ret стоит, а хотя бы 5..10 инструкции, которые работают с стеком.

А проверка на 0 никакие вызовы не требует. Это по сути две инструкции и выполняются они очень быстро.

Сложно представить, для чего может потребоваться:

  1. Вызов free с нулевым указателем в 99% случаев

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

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

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

А если функция возвращает объект, который должен быть сохранён в дереве или графе, и используется в нескольких местах. Кто должен удалить объект? «Тот кто создал — тот и удаляет», то есть та самая функция CreateObject? Я даже не представляю какова должна быть сигнатура функции CreateObject, которая одновременно и возвращает объект, и применяется для удаления, вводя в заблуждение одним своим названием.

Как вариант сохранить, через копирование. Это все равно будет быстрее.

Это приведёт к созданию другого объекта, а не оригинального. И если кто‐то, держащий ссылку на оригинал, изменит его, это не отразится на копии.

А как это возможно в обсуждаем случае?
Речь же про созданный объект в рамках исполнения функции. Никто другой не может иметь ссылку на этот объект до того как его вернет функция.

Я не только обнуляю указатели после free(), но и выполняю assert() перед free(), проверяя указатель на null. И знаете ли, помогало уже не раз.

Как именно помогло? Мне на самом деле интересно. Если free молчком проглатывает null как узнать, что помогло?

Вопрос не в том, что free() молча проглатывает null. Вопрос в том, что если предпринимается попытка освободить память, которая раньше не была выделена, то это признак ошибки в коде.

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

Всё равно не понятно, как ассерт проверяет что кусок памяти не освобожден? Если был занулен, то и ассерт не нужен. В статье вроде разбирается кейс с нулом и дабл фри

Ответили ниже, теперь понятно.

Пожалуй, этот комментарий заминусовали зря. Защитная методика программирования с использованием assert и очисткой указателя имеет смысл. Причём совместно это работает лучше, чем по отдельности. Просто нужно уточнить некоторые детали.

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

void foo(const char *name)
{
    char *buf = NULL;

    if (name == NULL)
        goto end;
    int64_t size = GetFileSize(name);
    if (size == -1)
        goto end;
    buf = (char *)malloc(size);
    if (buf == NULL)
        goto end;
    if (ReadFile(name, buf, size) == -1)
        goto end;
    Do(buf, size);
end:
    free(buf);
}

Здесь assert писать перед free смысла нет. Вполне естественно, что файл отсутствует и буфер для его чтения не понадобился. Если же это ненормальная ситуация и файл должен всегда быть, то логичнее assert написать выше рядом с кодом работы с файлом.

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

void foo(const char *name)
{
    if (name == NULL)
        return;

    char *copy = strdup(name);
    if (copy == NULL)
        return;

    Do(copy);

    assert(copy);
    free(copy);
}

Здесь assert уместен, так как проверяет договорённость, что буфер был успешно выделен и при освобождении указатель не может быть нулевым. Если это так, то что-то явно пошло не так и надо с этим разобраться.

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

Вот реальный фрагмент кода из одного проекта, в котором я недавно нашёл ошибку с помощью PVS-Studio:

    free(desc->manifest);
    desc->manifest = NULL;
    free(desc->manifest_digest);
    desc->manifest_digest = NULL;
    free(desc->config);
    desc->config = NULL;
    free(desc->config_digest);
    desc->config_digest = NULL;
    free(desc->uncompressed_digest);      // <=
    desc->uncompressed_digest = NULL;     // <=
    free(desc->compressed_digest);
    desc->compressed_digest = NULL;
    free(desc->tag);
    desc->tag = NULL;
    free(desc->uncompressed_digest);      // <=
    desc->uncompressed_digest = NULL;     // <=
    free(desc->layer_file);
    desc->layer_file = NULL;

Рука программиста дрогнула и он два раза пытается освободить один и тот-же буфер (выделил строки комментариями).

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

Обратите внимание, что защитное программирование в виде обнуления указателя защитила от двойного освобождения памяти. Однако, если добавить assert то станет ещё лучше.

    assert(desc->manifest);
    free(desc->manifest);
    desc->manifest = NULL;

    assert(desc->manifest_digest);
    free(desc->manifest_digest);
    desc->manifest_digest = NULL;

    assert(desc->config);
    free(desc->config);
    desc->config = NULL;

    assert(desc->config_digest);
    free(desc->config_digest);
    desc->config_digest = NULL;

    assert(desc->uncompressed_digest);
    free(desc->uncompressed_digest);      // <=
    desc->uncompressed_digest = NULL;     // <=

    assert(desc->compressed_digest);
    free(desc->compressed_digest);
    desc->compressed_digest = NULL;

    assert(desc->tag);
    free(desc->tag);
    desc->tag = NULL;

    assert(desc->uncompressed_digest);    // Сработает этот assert!
    free(desc->uncompressed_digest);      // <=
    desc->uncompressed_digest = NULL;     // <=

    assert(desc->layer_file);
    free(desc->layer_file);
    desc->layer_file = NULL;

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

P.S. А вообще выглядят вот такие простыни C кода для ручного управления памяти ужасно. Умные указатели C++ это прям отрада души конечно.

Теперь ясно. Спасибо

Следует ли проверять указатель на NULL перед вызовом функции free?

Закон Беттериджа же.

Следует ли проверять указатель на NULL перед вызовом функции free?

А не окажется ли так, что человек который то проверяет, то не проверяет наличие/содержимое указателя/переменной, в какой-то момент запутается и забудет проверить там где проверять критически важно?

Настолько ли много ресурсов вы сэкономите за счёт это микрооптимизации, что это оправдает увеличение вероятности ошибки в чём-то более важном?

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

Если бы люди не были рукожопами совершающими идиотские ошибки, то Хабр остался бы без большей части контента, а PVS Studio вообще без бизнеса.

ИМХО, автору стоит попробовать заняться популяризацией языка RUST.

Как это связано?

В комментах к статье про плюсы нужно обязательно упомянуть раст.

В переменную Z будет записано не 20, а 21, так как к моменту выбора переменной Y произойдёт её инкремент.

А не 22?

Будет 21

({ typeof (a) _a = (a);

Сто тысяч лет на плюсах не писал - в таком коде разве не сработает конструктор копирования (если передать обьект)?

Какие конструкторы... Это Си :)

free() — не единственная функция в libc, принимающая указатель и освобождающая его. Есть ещё несколько подобных функций: freeaddrinfo(), freehostent() и др.

Функция freeaddrinfo() в musl-libc устроена вот так:

void freeaddrinfo(struct addrinfo *p)
{
	size_t cnt;
	for (cnt=1; p->ai_next; cnt++, p=p->ai_next);
	struct aibuf *b = (void *)((char *)p - offsetof(struct aibuf, ai));
	b -= b->slot;
	LOCK(b->lock);
	if (!(b->ref -= cnt)) free(b);
	else UNLOCK(b->lock);
}

Она не проверяет указатель, который ей передали в качестве параметра, на NULL.

А проверка на NULL прямо внутри функции это скорее правило или скорее исключение?

Почитал комментарии и вспомнил, что не всё так однозначно. Помнится мне, что я приезжал, когда пользовался new и delete. Точно не помню компилятор, но что-то было, когда я пользовался delete с нулевым указателем. Код может и избыточен получается, но надёжней.

Зарегистрируйтесь на Хабре, чтобы оставить комментарий