void Execute(const char* sql_statement);
Бывают и случаи передачи объектов и примитивных типов по указателю.
Мне хотелось бы рассказать про то, что наверняка делают многие программисты на C и C++: проверяют входной параметр-указатель на NULL (nullptr):
void Execute(const char* sql_statement) {
if (!sql_statement) throw std::invalid_argument("Null SQL Statement");
...
}
Кроме варианта с исключением есть так же вариант с
assert(sql_statement)В данном случае оба варианта, скорее всего, ошибочны по своей сути и подлежат удалению из кода.
Причина здесь проста: указатель может содержать множество «невалидных» значений.
Если вы считаете, что указатель невалиден только когда его значение NULL (nullptr), то вы ошибаетесь.
Ассемблер
Давайте взглянем на ассемблерный код следующего куска кода:
volatile int* p = nullptr; //
*p = 0xDEADBEEF; //mov DWORD PTR ds:0, 0
// ud2
Мой компилятор — gcc 4.9.0. Платформа — Intel x64.
В результате мы получаем инструкцию ud2 — Undefined Instruction, что логично — по стандарту разыменование нулевого указателя — неопределенное поведение.
Давайте взглянем на следующий код:
volatile int* p = reinterpret_cast<volatile int*>(13); //
*p = 0xDEADBEEF; // mov DWORD PTR ds:13, -559038737
По адресу ds:13 пытаемся записать число -559038737. Скорее всего, если вы скомпилируете программу и запустите ее, вы получите Segmentation fault, так как по адресу 13 для вашей программы не выделено страниц памяти.
Таким образом почти любую функцию, которая проверяет валидность указателя можно обмануть, просто передав туда 13, хоть даже 4, 8, 15, 16, 23, 42, в общем, тот адрес, где для вас не выделено необходимых страниц памяти.
Так зачем стоит проверять на nullptr?
Проверять на NULL стоит в случае, если вы имеете опциональное значение, которое может быть, а может и не быть:
void Execute(const char* sql_statement, void (*callback)(State state)) {
…
if (callback) callback(OK);
}
Вывод: не стоит проверять указатель на NULL кроме тех случаев, когда параметр является опциональным.
Комментарии (20)
DancingOnWater
24.08.2015 21:47+6Честно говоря не понял пассажа. Assert не выполняются в релизном коде. а вот в дебажном, позволяют отловить неверное использование метода или функции.
Иными словами assert-ы надо применять максимально широко.
WinPooh73
24.08.2015 21:51+3> Иными словами assert-ы надо применять максимально широко.
Согласен. Но не заменять ими проверки, которые должны обеспечивать безопасность релизной версии.
Door
24.08.2015 21:55+3assert(sql_statement)
Не согласен с вами.assert()нужно использовать там, где есть подозрение, что может ошибиться программист.assert()сработает во время тестирования/разработки — когда код наиболее часто меняется. Но это не самое важное. Важно то, чтоassert()прекрасно ещё и документирует кусок кода:
void Execute(const char* sql_statement) { assert(sql_statement); }
сразу видно, чтоsql_statement— не опциональный, а обязательный параметр.
Таким образом почти любую функцию, которая проверяет валидность указателя можно обмануть, просто передав туда 13, хоть даже 4, 8, 15, 16, 23, 42, в общем, тот адрес, где для вас не выделено необходимых страниц памяти.
А то о чём вы говорите, как по мне, никак не относится к тому, что не нужно проверять указатель на валидность. Такого рода проблемы — почти во всех случаях — работа с уже удалённым объектом. И вообще, «обмануть в плюсах» можно на каждом шагу:
struct Foo { void bar() { std::puts("bar"); } }; static_cast<Foo*>(nullptr)->bar();
Все знают, что здесь UB, а также все знают, что не один компилятор, в этом случае, не упадёт и программа спокойно напечатает bar. Но это не значит, что такой код можно писать.
monolithed
25.08.2015 00:11std::unique_ptr<Foo>()->bar();
А такой можно?)
mapron
25.08.2015 02:13Вы еще виртуальной объявите bar(), тогда не будете сомневаться, можно-не можно) не можно потому, что могут появиться обращения к полям и виртуальные методы. Не можно с точки зрения здравого смысла. unique_ptr лишь меняет внешний вид кода =) Может в дебажной реализации будет какой-то ASSERT в operator ->, исходники смотреть надо.
alafix
24.08.2015 22:49+1Что-то никак не получается сделать выводы из прочитанного. Автор призывает отказаться от проверок на NULL(nullpr), заменив его проверкой на валидность? А что тогда понимать под термином «валидность»? Ведь ни для кого не секрет, что с точки зрения ОС в 99% случаев валиден будет даже указатель, по которому только что прошлись delete (free) — страницы памяти-то уже выделены. С этой же точки зрения неинициализированный указатель тоже вполне может быть валиден. Удивляется дядя Вася…
Lol4t0
24.08.2015 22:52+4Надо совсем избавляться от сырых указателей
void Execute(const std::string& sql_statement);
В огромном большинстве случаев это сделать можноalafix
24.08.2015 22:59Как раз в этом примере скармливать этот самый sql_statement драйверу СУБД в огромном большинстве случаев придется именно в виде указателя. Хотя мы в этом случае очищаем совесть.
Error1024
25.08.2015 05:29В C при невозможности выделить память вернется NULL, поэтому думаю проверка здесь не лишняя.
fareloz
25.08.2015 10:53+1Как-то все в одну кучу смешалось — NULL и nullptr. Лично для меня это не одно и то же.
berez
В большинстве случаев невалидный указатель как раз-таки нулевой. По моим прикидкам — где-то в 90% случаев.
Несколько раз видел ситуацию, когда указатель невалидный, но ненулевой. Обычно это получается в результате:
1) использования указателя на потерянный/удаленный объект
2) приведения нулевого указателя к указателю на подобъект (в т.ч. неявное)
3) один раз(!) видел вживую неинициализированный указатель.
В общем, ваш совет не очень хороший. Лучше расставить ассерты и получать ошибки сразу же в месте их возникновения, чем пытаться разобраться в крэш-дампе, когда нулевой указатель пролетел через несколько вложенных вызовов функций и свалил программу в самом непонятном месте.
belator
1) Устраняется с помощью unique_ptr и метода get(), как рассказывал Герб Саттер.
3) Ставить опции компилятора на неинициализированные переменные.
assert'ы вещь хорошая, надо только хорошо им уметь пользоваться, что не всегда возможно в команде, состоящей из множества человек разного уровня.
Assert-based-design зачастую бывает не самым лучшим решением, когда разработка идет достаточно быстро.
Imp5
Не надо давать писать на C++ человеку, который не умеет использовать assert().
Mihasik1984
Я бы добавил, что не надо давать писать на C++ человеку, который вообще не умеет правильно работать с динамической памятью.
WinPooh73
Поведение assert, по стандарту, зависит от определённости макроса NDEBUG. В релизной версии он просто не сработает, и нулевой указатель пойдёт гулять по вызовам вложенных функций.
gbg
То, что невалидный указатель чаще всего нулевой следует как раз из другого совета:
Обнуляйте опустошенные указатели.
Вкупе с тем, что
deleteпо стандарту [$5.3.5/2] игнорирует обнуленные указатели, это хороший способ дважды не удалить что-либо.Lol4t0
delete у вас в коде вообще не должен встречаться!
gbg
С этим я полностью согласен. Можно даже ужесточить — в коде крайне нежелательны «сырые» указатели, не обернутые во что-то автоматическое/умное/гениальное.
gurinderu
Предлагаете все завернуть в smart pointers и потерять в производительности?