Логическое выражение в программировании — конструкция языка программирования, результатом вычисления которой является «истина» или «ложь». Во многих книгах по программированию, предназначенных для изучения языка «с нуля», приводятся возможные операции над логическими выражениями, с которыми сталкивался каждый начинающий разработчик. В этой статье я не буду рассказывать, что оператор 'И' приоритетнее оператора 'ИЛИ'. Я расскажу о распространённых ошибках в простых условных выражениях, состоящих всего из трёх операторов, и покажу, как можно проверить свой код с помощью построения таблиц истинности. Описанные ошибки делают разработчики таких известных проектов как FreeBSD, Microsoft ChakraCore, Mozilla Thunderbird, LibreOffice и многих других.Введение
Я занимаюсь разработкой статического анализатора кода для языков C/C++/C# — PVS-Studio. В моей работе приходится много сталкиваться с открытым и закрытым кодом разных проектов. Часто результатом такой работы являются статьи о проверке open source проектов, содержащие описание найденных ошибок и недочётов. После просмотра большого объёма кода начинаешь замечать различные паттерны ошибок, которые допускают программисты. Так, мой коллега Андрей Карпов писал статью про эффект последней строки после нахождения большого количества ошибок, допущенных в последних фрагментах однотипного кода.
В начале этого года я проверил с помощью анализатора много проектов крупных компаний в сфере IT, которые, следуя современной тенденции, выкладывают в открытый доступ исходный код своих проектов под свободными лицензиями. Я стал замечать, что почти в каждом проекте находится ошибка в условном выражении из-за неправильной записи условных операторов. Само выражение достаточно простое и состоит всего из трёх операторов:
- != || !=
- == || !=
- == && ==
- == && !=
Всего таких условных выражений можно записать 6 штук, но 4 из них являются ошибочными: два являются всегда истинным или ложным; в двух результат всего выражения не зависит от результата входящего в него подвыражения.
Для доказательства неверного результата выражения я буду строить таблицу истинности в каждом примере; также я приведу для каждого примера по одному фрагменту кода из открытого проекта. В этой статье будет упомянут и тернарный оператор '?:', который имеет почти самый низкий приоритет из всех операторов, но очень много разработчиков не знают об этом.
Т.к. чаще всего я встречал неправильные условные выражения при проверке результата разных функций, код возврата которых сравнивают с кодами ошибок, то в приводимых далее синтетических примерах я буду использовать переменную с именем err, а code1 и code2 будут константами. При этом константы code1 и code2 не равны. Значение «other codes» будет означать любые другие константы, не равные code1 и code2.
Ошибки с использованием оператора '||'
Выражение != || !=
Синтетический пример, в котором результат условного выражения всегда будет равен истине:
if ( err != code1 || err != code2)
{
....
}Далее представлена таблица истинности для этого примера кода:
Теперь посмотрим на реальный пример ошибки, найденной в проекте LibreOffice.
V547 Expression is always true. Probably the '&&' operator should be used here. sbxmod.cxx 1777
enum SbxDataType {
SbxEMPTY = 0,
SbxNULL = 1,
....
};
void SbModule::GetCodeCompleteDataFromParse(
CodeCompleteDataCache& aCache)
{
....
if( (pSymDef->GetType() != SbxEMPTY) || // <=
(pSymDef->GetType() != SbxNULL) ) // <=
aCache.InsertGlobalVar( pSymDef->GetName(),
pParser->aGblStrings.Find(pSymDef->GetTypeId()) );
....
}Выражение == || !=
Синтетический пример, в котором результат всего условного выражения не зависит от результата подвыражения (err == code1):
if ( err == code1 || err != code2)
{
....
}Далее представлена таблица истинности для этого примера кода:
Теперь посмотрим на реальный пример ошибки, найденной в проекте FreeBSD.
V590 Consider inspecting the 'error == 0 || error != — 1' expression. The expression is excessive or contains a misprint. nd6.c 2119
int
nd6_output_ifp(....)
{
....
/* Use the SEND socket */
error = send_sendso_input_hook(m, ifp, SND_OUT,
ip6len);
/* -1 == no app on SEND socket */
if (error == 0 || error != -1) // <=
return (error);
....
}Не сильно он отличается от синтетического примера, не правда ли?
Ошибки с использованием оператора '&&'
Выражение == && ==
Синтетический пример, в котором результат условного выражения всегда будет ложным:
if ( err == code1 && err == code2)
{
....
}Далее представлена таблица истинности для этого примера кода:
Теперь посмотрим на реальный пример ошибки, найденной в проекте SeriousEngine.
V547 Expression is always false. Probably the '||' operator should be used here. entity.cpp 3537
enum RenderType {
....
RT_BRUSH = 4,
RT_FIELDBRUSH = 8,
....
};
void
CEntity::DumpSync_t(CTStream &strm, INDEX iExtensiveSyncCheck)
{
....
if( en_pciCollisionInfo == NULL) {
strm.FPrintF_t("Collision info NULL\n");
} else if (en_RenderType==RT_BRUSH && // <=
en_RenderType==RT_FIELDBRUSH) { // <=
strm.FPrintF_t("Collision info: Brush entity\n");
} else {
....
}
....
}Выражение == && !=
Синтетический пример, в котором результат всего условного выражения не зависит от результата подвыражения «err != code2»:
if ( err == code1 && err != code2)
{
....
}
Далее представлена таблица истинности для этого примера кода:
Теперь посмотрим на реальный пример ошибки, найденной в проекте ChakraCore — JavaScript-движке для Microsoft Edge.
V590 Consider inspecting the 'sub[i] != '-' && sub[i] == '/'' expression. The expression is excessive or contains a misprint. rl.cpp 1388
const char *
stristr
(
const char * str,
const char * sub
)
{
....
for (i = 0; i < len; i++)
{
if (tolower(str[i]) != tolower(sub[i]))
{
if ((str[i] != '/' && str[i] != '-') ||
(sub[i] != '-' && sub[i] == '/')) { / <=
// if the mismatch is not between '/' and '-'
break;
}
}
}
....
}Ошибки с использованием оператора '?:'
V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '|' operator. ata-serverworks.c 166
static int
ata_serverworks_chipinit(device_t dev)
{
....
pci_write_config(dev, 0x5a,
(pci_read_config(dev, 0x5a, 1) & ~0x40) |
(ctlr->chip->cfg1 == SWKS_100) ? 0x03 : 0x02, 1);
}
....
}В заключение хочу сказать про тернарный оператор '?:'. Его приоритет почти самый низкий среди всех операторов. Ниже только у присваивания, throw и оператора «запятая». Приведённая в примере ошибка была найдена в ядре FreeBSD. Здесь тернарным оператором воспользовались для выбора нужного флажка и чтобы написать короткий красивый код. Но приоритет оператора побитового 'ИЛИ' выше, поэтому условное выражение вычисляется не в том порядке, в каком планировал программист. Эту ошибку я тоже решил описать в этой статье, т.к. она является очень распространённой среди проверенных мною проектов.
Заключение
Описанные шаблоны условных выражений могут нести большую опасность, если не проявлять повышенную внимательность при написании кода. Несмотря на небольшое количество операторов, всё условное выражение может пониматься неправильно. Код, в котором допустили такую ошибку, может выглядеть вполне логичным и будет пропущен после code-review. Подстраховать себя от таких ошибок можно путём проверки своего кода с помощью построения таблиц истинности, если есть сомнения в правильности условия, а также с помощью регулярных проверок статическими анализаторами.
Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Svyatoslav Razmyslov. Logical Expressions in C/C++. Mistakes Made by Professionals .
Комментарии (45)
andreili
11.04.2016 10:31+5По поводу тернарного оператора — уже давно вошло в привычку брать его в скобки. И условие в свои отдельные скобки. ИМХО — это должно войти в привычку у многих. Иначе от граблей из-за различных приоритетов долго будут страдать все…
И еще — когда идут сложные условия ветвления (больше 1), рекомендуется хоть как-то составить таблицу истинности — никто не застрахован от ошибок, а так за счет пары лишних минут можно прилично уменьшить вероятность создания бага.
zirf
11.04.2016 10:41+1Я вообще даже если можно и без скобок/кавычек все равно их ставлю. Это «можно» вроде как к естественному языку должно приближать, но по мне так и вероятность ошибок повышает.
SvyatoslavMC
11.04.2016 10:57+6if( (pSymDef->GetType() != SbxEMPTY) || (pSymDef->GetType() != SbxNULL) )
Наличие скобок в таких примерах не сильно помогает не допустить ошибку. С приоритетом операций в этом примере тоже всё понятно. Я хотел сделать акцент именно на необычные комбинации трёх операторов. Нужно просто запомнить, что такие ситуации бывают.
LoadRunner
11.04.2016 13:42Жаль, что не было акцента на двух правильных вариантах. Ведь если условное выражение не совпадает с одним из этих вариантов, то значит оно неверно составлено.
SvyatoslavMC
11.04.2016 14:28Вы правильно сказали, что если условное выражение совпадает с одним из этих вариантов, то, скорее всего, оно составлено неверно.
А вот о правильных вариантах почти невозможно рассуждать, потому что у каждого будет своя логика в программе.
Например, в условии ниже есть ошибка — оно всегда истинно:
if ( err != code1 || err != code2) { .... }
Сделав замену оператора || на && мы получим такое условие:
if ( err != code1 && err != code2) { .... }
которое будет истинно для любых значений err, не равных code1 и code2. В этом синтетическом примере отсутствует исходная логическая ошибка, но мы не можем утверждать, что это будет правильной правкой в каждом конкретном проекте. А условия, которые не зависят от части логического подвыражения, непонятно даже как исправить на синтетическом примере, не то, что в реальном коде.
LoadRunner
11.04.2016 14:55Я несколько о другом говорил. Не знаю, как у всех, но части людей нельзя говорить про то, как делать нельзя, но не говорить про то, как делать можно (хотя оно логически вытекает из запрета). Простой пример: мать посылает сына в магазин за хлебом и говорит: «Купи любой, только батон нарезной не покупай». И пока он собирается и одевается, постоянно напоминает — «не покупай нарезной». Приходит сын в магазин и вспоминает — «Какой хлеб купить надо было? А! Мама постоянно про батон нарезной говорила, куплю его.»
Я не рассуждаю о том, что два оставшихся варианта всегда правильны. Но статья нам доказывает, что перечисленные четыре — всегда избыточны в количестве условий. Поэтому, если наше условное выражение не подходит под != && != или == || ==, то оно заведомо содержит в себе лишние условия.
Кому как, но мне проще запомнить два верных в большинстве случаев выражения, чем запоминать четыре, которые содержат лишнее условие.
zirf
11.04.2016 10:38-3Errare humanum est. Кстати, ошибки в логических выражениях внесены еще в ВУЗе прямо в головы разработчиков. Как и многие другие.
Есть еще один хитрый источник — особенности самого процесса, не вдаваясь в физиологию ВНД скажу, что процесс идет с очень непостоянной скоростью. Естественно, концентрация тоже разная. Но это вопрос очень темный.
zagayevskiy
11.04.2016 20:25+5Не понял пассажа про ошибки и ВУЗ, поясните, пожалуйста.
zirf
11.04.2016 21:18-6Не обязательно ВУЗ. Но, классически одна из задач ВУЗа выработать навыки мышления и способность к обучению (я именно такой заканчивал), а не просто дать знания (бесполезные к окончанию). То есть если человек системно ошибается, навыки у него не выработаны, просто я видел как массово ошибались выпускники одного ВУЗа, что то не то было в обучении (известно что, но там вопрос религии). Интереснее второй момент. Людям свойственно ошибаться. ВУЗ не при чем, чистая психология
>--------<
<-------->
Нарисуйте это на бумаге сплошными линиями, очевидно, линии одной длинны, но на рисунке так не покажется! Значки меняют субъективное восприятие. Так же, полагаю, есть ситуации, когда человек ошибется с большой долей вероятности за счет фокуса со стрелками. Картина в мозге — не объективная реальность, а восприятие конкретным индивидуумом. А ВУЗ… ну люблю некоторых обучателей чихвостить, им в голову не приходит, что 18-летний все принимает на веру и для него это будет истина до конца жизни на уровне подсознания.
Charg
11.04.2016 23:20+5Эмм, а что это за поток сознания? Где кому какие ошибки в вузе внушают\развивают?
zirf
13.04.2016 09:31-1Профессиональные.
Charg
15.04.2016 09:20Это прям какой-то конкурс на звание «мистер-неопределенность».
Можно больше конкретики?zirf
15.04.2016 15:08Вы не обратили внимание, «Как ошибаются профессионалы» — заголовок, но кто-то определенно сказал почему?
Если интересно — мне есть смысл пытаться что-то объяснить, нет, скучно смотреть на ведро минусов.
Я готов услышать, что неверного в утверждении «Errare humanum est».SvyatoslavMC
15.04.2016 16:40Как в вашу теорию вписываются зарубежные разработчики, участвующие и делающие ошибки в открытых проектах? В западных вузах совсем иной подход к обучению программированию.
mafia8
11.04.2016 10:39Осталось узнать, как оно работает при таком коде.
khim
11.04.2016 11:44+1Defensive programming. То, что не отловится одной проверкой заметит другая… скорее всего.
С одной стороны даёт возможность создавать современных монстров на сотни миллионов строк кода, а с другой — гарантирует что дыры в безопасности будут «лататься» вечно.
Чтобы дыр не было нужно писать fail-fast/crash-only software и/или использовать GIGO (тогда проверок в системе остаётся мало и каждую из них можно хорошо обдумать), но это замедляет разработку кода, потому так никто не делает…
PeterAlmazov
11.04.2016 11:49+8«Подстраховать себя от таких ошибок можно путём проверки своего кода с помощью построения таблиц истинности.»
Есть еще один метод — выучить, наконец, законы де Моргана.
Fil
11.04.2016 12:01+15Ага, с их помощью не очень очевидное
err != code1 || err != code2
становится более понятным
!(err == code1 && err == code2)
sborisov
11.04.2016 11:53+1#include <iostream> enum ERROR{ code1 = 0, code2 = 1, code3 = 2 }; int main(int argc, char const *argv[]) { ERROR err = code3; if ( err == code1 || err == code2) { std::cout<< "err equal code1 or code2: "<<err<<std::endl; } else { std::cout<< "err is: "<<err<<std::endl; } return 0; }
g++ log.cpp
./a.out
err is: 2
Логическое условие выполняется, как и задумано. Т.е. выполняется ветвь кода, когда ERROR err = code3sborisov
11.04.2016 12:01-2Т.е. в таких ситуациях писать switch безопаснее. Но длиннее…
Ещё можно было напомнить, что, когда компилятор не вычисляет правую часть выражения, при определённых условиях в || и &&
sborisov
11.04.2016 12:04В случае, если заменить условие
if ( err != code1 || err != code2)
Компилятор вычислив левую часть выражения и получив true — правую (после оператора || )вычислять уже не будет, и вывод программы будет ошибочным
#include <iostream> enum ERROR{ code1 = 0, code2 = 1, code3 = 2 }; int main(int argc, char const *argv[]) { ERROR err = code3; if ( err != code1 || err != code2) { std::cout<< "err equal code1 or code2: "<<err<<std::endl; } else { std::cout<< "err is: "<<err<<std::endl; } return 0; }
g++ log.cpp
./a.out
err equal code1 or code2: 2
sborisov
11.04.2016 12:14-1В предыдущем комментарии забыл поправить иходник.
Тут видя false в первой половине, компилятор вычисляет вторую и она оказывается true — при || итог всего условия true, хотя err == code1
#include <iostream> enum ERROR{ code1 = 0, code2 = 1, code3 = 2 }; int main(int argc, char const *argv[]) { ERROR err = code1; if ( err != code1 || err != code2) { std::cout<< "err isn't equal code1 or code2: "<<err<<std::endl; } else { std::cout<< "err is: "<<err<<std::endl; } return 0; }
g++ log.cpp
./a.out
err isn't equal code1 or code2: 0
SvyatoslavMC
11.04.2016 12:43Это всё понятно. Только мало кто во время работы пишет экспериментальные программки.
tangro
11.04.2016 15:42+1На каждом шагу. Ну, для проверки двух условий в ифе — нет, конечно, но перед тем, как притянуть в проект какой-то паттерн или либу — почти каждый раз.
olekl
11.04.2016 14:26А еще наверное частая ошибка когда без скобочек используют маски и сравнения вроде if ( a & 3 == b & 3), нет?
SvyatoslavMC
11.04.2016 15:16Я повидал много частых ошибок) Возможно, про флаги и маски можно будет свою историю рассказать, если поизучать всю нашу базу ошибок.
Yuuri
11.04.2016 15:10+1А эта проверка только для простых типов проводится, или для всех? А то с C++-ной возможностью перегрузить операторы можно какое угодно поведение получить, и описанные случаи окажутся вполне корректными. Например, если делается интерпретатор/интероп для некоего слаботипизированного языка программирования, где "" == 0 && "" == false – true. Или eDSL с компактным синтаксисом, вроде boost::spirit, где старые операторы наделяют другой семантикой из-за невозможности добавлять новые.
SvyatoslavMC
11.04.2016 15:35Анализатор ищет неправильные конструкции, какие были приведены в примерах. Но как и во всех диагностиках, для правил V547 и V590 реализован ряд своих исключений, которые позволяют избавить диагностику от ложных срабатываний. Я сейчас не вспомню все исключения, тем более для двух правил. Скорее всего, благодаря им я не встречал ложные срабатывания в описанных вами ситуациях, зато встречал много ошибок на простом коде.
ForNeVeR
11.04.2016 15:48+1Что также может быть интересно — для пользовательских операторов не работает short-circuit-поведение.
ASTAPP
11.04.2016 18:05Если речь идет не о элементарных типах, а о классах, оператор сравнения для которых может быть перегружен, то конструкция
if (en_RenderType==RT_BRUSH &&
en_RenderType==RT_FIELDBRUSH)
Может быть не ложной.khim
11.04.2016 18:11+3Есть много способов выстрелить себе в ногу. Есть целые языки, устроенные так, чтобы люди, их использующие страдали до тех пор, пока не наберутся опыта и не перейдут на что-то менее ужасное. Но… Зачем? Равенство должно быть отношением эквиватентности, незачем из него делать невесть что.
win32asm
12.04.2016 09:06понятно, что в
if (err == ERR_OK || err != ERR_PENDING)…
можно спокойно убрать кусок с ERR_OK, но КМК иногда это может быть «документацией», что обрабатываем, например, «хороший» и «плохие» сценарии, а «злой», в смысле асинхронный, будет, например, где-то ещё.
Что в общем не должно мешать оторвать руки человеку, придумавшему такой API. 8-)
gena_glot
12.04.2016 09:063 и 4 это ошибки. 1 и 2 нет.
!= || !=
== || !=
Оба случая — может достаточно наличия/отсутствия одного из двух признаков. Не согласен совершенно с автором статьи.SvyatoslavMC
12.04.2016 09:12+1Ну прям совершенно не согласны?) Это же реальные баги и реально ничего не делающий код.
Оба случая — может достаточно наличия/отсутствия одного из двух признаков.
Это как раз причина того, почему такие ошибки долго не замечают.
Zavtramen
Не совсем понятно почему упор сделан именно на ошибки с C/C++. Это достаточно распространенный синтаксис и встречается во многих языках.
vlivyur
Ага, сам постоянно туплю, когда одно из условий с отрицанием (когда оба, тогда проще — надо составить условие без отрицаний, а потом || на && и наоборот заменить). Правда == && == у себя в ошибках не встречал.
qw1
Я когда делаю рефакторинг с вынесением отрицаний наружу или внесением внутрь, не задумываюсь о смысле выражения, а просто механически применяю формулы. Ошибок не возникает.
Jesus05
Видимо потому что PVS-studio чаще всего C\C++ проекты проверяет открытые. А так да, проблема с операторами думаю для всех языков актуальна.