Комментарии 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
-ы добавлять для экономии на вызовах функций.
Тогда есть смысл подумать в сторону, чтобы вообще не проверять указатели. Например, создать свой менеджер памяти, который будет учитывать редкие выделения памяти и хранить список таких буферов где-то отдельно. Потом скопом очищать их, когда они больше не нужны.
Это просто пример. Можно что-то ещё придумать. Смысл в том, что разумно оптимизировать такие ситуации с верхнего уровня, а не снизу по мелочи. Не там ищется решение задачи оптимизации.
Да блин, представьте, что вы пишете деструктор дерева. Хоть двоичного, хоть В-звездатого. В нём всегда нулевых указателей будет больше, чем ненулевых (примерно в В раз в лучшем случае)
Представляю :) У нас как раз деревья. И решение для повышения эффективности - освобождать всё сразу :)
Какая стратегия освобождения памяти используется в C и С++ ядре PVS-Studio?
Предполагаю, что она будет медленная только в том случае, если ptr все-таки не NULL.
Сложно представить, для чего может потребоваться:
Вызов free с нулевым указателем в 99% случаев
Это настолько критичный по времени код, что есть смысл проверять указатель перед вызовом 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?
А не окажется ли так, что человек который то проверяет, то не проверяет наличие/содержимое указателя/переменной, в какой-то момент запутается и забудет проверить там где проверять критически важно?
Настолько ли много ресурсов вы сэкономите за счёт это микрооптимизации, что это оправдает увеличение вероятности ошибки в чём-то более важном?
ИМХО, автору стоит попробовать заняться популяризацией языка RUST.
В переменную Z будет записано не 20, а 21, так как к моменту выбора переменной Y произойдёт её инкремент.
А не 22?
({ 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.
Почитал комментарии и вспомнил, что не всё так однозначно. Помнится мне, что я приезжал, когда пользовался new и delete. Точно не помню компилятор, но что-то было, когда я пользовался delete с нулевым указателем. Код может и избыточен получается, но надёжней.
Следует ли проверять указатель на NULL перед вызовом функции free?