Хорошей ли практикой является ОБНУЛЕНИЕ указателя после его удаления?

StackOverflow https://stackoverflow.com/questions/1931126

Вопрос

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

Каковы проблемы со следующим кодом?

Foo * p = new Foo;
// (use p)
delete p;
p = NULL;

Это было вызвано ответ и комментарии перейдем к другому вопросу.Один комментарий от Нил Баттеруорт сгенерировал несколько положительных голосов:

Установка указателей в значение NULL после удаления не является универсальной хорошей практикой в C ++.Бывают моменты, когда это полезно делать, и моменты, когда это бессмысленно и может скрывать ошибки.

Есть масса обстоятельств, когда это не помогло бы.Но, по моему опыту, это не повредит.Кто-нибудь, просветите меня.

Это было полезно?

Решение

Установка указателя на 0 (который является "нулевым" в стандартном C ++, определение NULL в C несколько отличается) позволяет избежать сбоев при двойных удалениях.

Рассмотрим следующее:

Foo* foo = 0; // Sets the pointer to 0 (C++ NULL)
delete foo; // Won't do anything

Принимая во внимание , что:

Foo* foo = new Foo();
delete foo; // Deletes the object
delete foo; // Undefined behavior 

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

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

Наконец, побочное замечание, касающееся управления распределением объектов, я предлагаю вам ознакомиться с std::unique_ptr для строгого / единичного владения, std::shared_ptr для совместного владения или другой реализации интеллектуального указателя, в зависимости от ваших потребностей.

Другие советы

Установка указателей в значение NULL после удаления того, на что они указывали, конечно, не повредит, но часто это своего рода пластырь для решения более фундаментальной проблемы:Почему вы вообще используете указатель?Я вижу две типичные причины:

  • Вы просто хотели, чтобы что-то было выделено в куче.В этом случае обернуть его в объект RAII было бы намного безопаснее и чище.Завершите область действия объекта RAII, когда он вам больше не понадобится.Вот как std::vector работает, и это решает проблему случайного оставления указателей на освобожденную память.Здесь нет никаких указателей.
  • Или, возможно, вам нужна была какая-то сложная семантика совместного владения.Указатель, возвращенный из new может быть, это не то же самое, что тот, который delete призывается.За это время несколько объектов могли использовать этот объект одновременно.В этом случае предпочтительнее был бы общий указатель или что-то подобное.

Мое эмпирическое правило заключается в том, что если вы оставляете указатели в пользовательском коде, вы делаете это неправильно.Указатель не должен быть там, чтобы указывать на мусор в первую очередь.Почему нет объекта, который взял бы на себя ответственность за обеспечение его достоверности?Почему его область действия не заканчивается, когда заканчивается объект, на который указывают?

У меня есть еще лучшая передовая практика:Там, где это возможно, завершите область действия переменной!

{
    Foo* pFoo = new Foo;
    // use pFoo
    delete pFoo;
}

Я всегда устанавливаю указатель на NULL (сейчас nullptr) после удаления объекта (ов), на который он указывает.

  1. Это может помочь отловить много ссылок на освобожденную память (при условии, что ваша платформа выдает ошибку из-за deref нулевого указателя).

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

  3. Это замаскирует двойное удаление, но я нахожу, что это гораздо менее распространено, чем доступ к уже освобожденной памяти.

  4. Во многих случаях компилятор собирается оптимизировать его.Так что аргумент о том, что в этом нет необходимости, меня не убеждает.

  5. Если вы уже используете RAII, то их не так много deleteначнем с того, что s присутствует в вашем коде, поэтому аргумент о том, что дополнительное назначение создает беспорядок, меня не убеждает.

  6. Часто при отладке удобно видеть нулевое значение, а не устаревший указатель.

  7. Если это все еще беспокоит вас, используйте вместо этого интеллектуальный указатель или ссылку.

Я также устанавливаю для других типов дескрипторов ресурсов значение no-resource, когда ресурс свободен (что обычно есть только в деструкторе RAII-оболочки, написанной для инкапсуляции ресурса).

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

Обновить: Корпорация Майкрософт считает, что это хорошая практика для обеспечения безопасности, и рекомендует ее в своих политиках SDL.По-видимому, MSVC ++ 11 будет растопчите удаленный указатель автоматически (во многих случаях), если вы компилируете с параметром /SDL.

Во-первых, существует множество существующих вопросов по этой и тесно связанным темам, например Почему delete не устанавливает указатель в значение NULL?.

В вашем коде проблема в том, что происходит внутри (используйте p).Например, если где-то у вас есть такой код, как этот:

Foo * p2 = p;

тогда установка p в значение NULL дает очень мало результатов, так как у вас все еще есть указатель p2, о котором нужно беспокоиться.

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

Если есть еще код после delete, Да.Когда указатель удаляется в конструкторе или в конце метода или функции, Нет.

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

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

Как говорили другие, delete ptr; ptr = 0; это не приведет к тому, что демоны вылетят у вас из носа.Тем не менее, это поощряет использование ptr в качестве своего рода флага.Код становится замусоренным delete и устанавливаем указатель на NULL.Следующий шаг - разбросать if (arg == NULL) return; с помощью вашего кода для защиты от случайного использования NULL указатель.Проблема возникает после проверки на NULL станьте вашим основным средством проверки состояния объекта или программы.

Я уверен, что где-то есть кодовый запах использования указателя в качестве флага, но я его не нашел.

Я немного изменю ваш вопрос:

Вы бы использовали неинициализированный указатель?Вы знаете, тот, которому вы не установили значение NULL или не выделили память, на которую он указывает?

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

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

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

Если у вас нет другого ограничения, которое заставляет вас либо устанавливать, либо не устанавливать указатель в значение NULL после его удаления (одно из таких ограничений было упомянуто Нил Баттеруорт), тогда мое личное предпочтение - оставить все как есть.

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

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

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

Если это то, что вы на самом деле имеете в виду, лучше сделать это явным в исходном коде, используя что-то вроде повышение:: необязательно

optional<Foo*> p (new Foo);
// (use p.get(), but must test p for truth first!...)
delete p.get();
p = optional<Foo*>();

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

Это ребенок во всей этой ванне с C ++, не стоит его выбрасывать.:)

В хорошо структурированной программе с соответствующей проверкой ошибок нет никаких причин нет присвоить ему значение null. 0 стоит особняком как общепризнанное недопустимое значение в этом контексте.Терпите неудачу изо всех сил и потерпите неудачу скоро.

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

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

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

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

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

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

Позвольте мне расширить то, что вы уже вложили в свой вопрос.

Вот что вы вложили в свой вопрос в виде маркера:


Установка указателей в значение NULL после удаления не является универсальной хорошей практикой в C ++.Бывают моменты, когда:

  • это хороший поступок
  • и времена, когда это бессмысленно и может скрывать ошибки.

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

Так что, если вы сомневаетесь, просто обнулите его.

Сказав это, если вы чувствуете, что вам нужно явно обнулить какой-то указатель, то для меня это звучит так, как будто вы недостаточно разделили метод и должны взглянуть на подход рефакторинга, называемый "Метод извлечения", чтобы разделить метод на отдельные части.

ДА.

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

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

Я всегда использую макрос для удаления:

#define SAFEDELETE(ptr) { delete(ptr); ptr = NULL; }

(и аналогично для массива, free(), освобождающего дескрипторы)

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

static void TreeItem::DeleteSubtree(TreeItem *&rootObject)
{
    if (rootObject == NULL)
        return;

    rootObject->UnlinkFromParent();

    for (int i = 0; i < numChildren)
       DeleteSubtree(rootObject->child[i]);

    delete rootObject;
    rootObject = NULL;
}

Редактировать

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

Есть также много способов, которыми вы могли бы реализовать вышеизложенное - я просто пытаюсь проиллюстрировать идею принуждения людей ОБНУЛЯТЬ указатель, если они удаляют объект, вместо того, чтобы предоставлять им средства для освобождения памяти, которая не ОБНУЛЯЕТ указатель вызывающего объекта.

Конечно, приведенный выше пример - это всего лишь шаг к автоматическому указателю.Чего я не предлагал, потому что OP специально спрашивал о случае отказа от использования автоматического указателя.

"Бывают моменты, когда это полезно делать, и моменты, когда это бессмысленно и может скрывать ошибки".

Я вижу две проблемы:Этот простой код:

delete myObj;
myobj = 0

становится основой в многопоточной среде:

lock(myObjMutex); 
delete myObj;
myobj = 0
unlock(myObjMutex);

"Лучшая практика" Дона Нойфельда применима не всегда.Например.в одном автомобильном проекте нам пришлось устанавливать указатели на 0 даже в деструкторах.Я могу себе представить, что в критически важном для безопасности программном обеспечении такие правила не редкость.Проще (и мудрее) следовать им, чем пытаться убедить команду / code-checker для каждого использования указателя в коде, что строка, обнуляющая этот указатель, избыточна.

Другая опасность заключается в том, что мы полагаемся на этот метод в коде, использующем исключения:

try{  
   delete myObj; //exception in destructor
   myObj=0
}
catch
{
   //myObj=0; <- possibly resource-leak
}

if (myObj)
  // use myObj <--undefined behaviour

В таком коде либо вы приводите к утечке ресурсов и откладываете решение проблемы, либо процесс завершается сбоем.

Итак, эти две проблемы, спонтанно возникшие у меня в голове (Херб Саттер наверняка рассказал бы больше), делают для меня все вопросы типа "Как избежать использования смарт-указателей и безопасно выполнять работу с обычными указателями" устаревшими.

Всегда есть Свисающие Указатели о чем беспокоиться.

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

Как многие уже говорили, конечно, гораздо проще просто использовать умные указатели.

Редактировать:Как сказал Томас Мэтьюз в этот более ранний ответ, если указатель удален в деструкторе, нет никакой необходимости присваивать ему значение NULL, поскольку он больше не будет использоваться, поскольку объект уже уничтожается.

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

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

shared_ptr<Foo> p(new Foo);
//No more need to call delete

Он выполняет подсчет ссылок и является потокобезопасным.Вы можете найти его в пространстве имен tr1 (std::tr1, #include < память >) или, если ваш компилятор не предоставляет его, получите его из boost.

Лицензировано под: CC-BY-SA с атрибуция
Не связан с StackOverflow
scroll top