Логические выражения в C/C++. Как ошибаются профессионалы

    Логическое выражение в программировании — конструкция языка программирования, результатом вычисления которой является «истина» или «ложь». Во многих книгах по программированию, предназначенных для изучения языка «с нуля», приводятся возможные операции над логическими выражениями, с которыми сталкивался каждый начинающий разработчик. В этой статье я не буду рассказывать, что оператор 'И' приоритетнее оператора 'ИЛИ'. Я расскажу о распространённых ошибках в простых условных выражениях, состоящих всего из трёх операторов, и покажу, как можно проверить свой код с помощью построения таблиц истинности. Описанные ошибки делают разработчики таких известных проектов как 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 .
    PVS-Studio
    1,160.07
    Static Code Analysis for C, C++, C# and Java
    Share post

    Comments 46

      +2
      Не совсем понятно почему упор сделан именно на ошибки с C/C++. Это достаточно распространенный синтаксис и встречается во многих языках.
        0
        Ага, сам постоянно туплю, когда одно из условий с отрицанием (когда оба, тогда проще — надо составить условие без отрицаний, а потом || на && и наоборот заменить). Правда == && == у себя в ошибках не встречал.
          +4
          Я когда делаю рефакторинг с вынесением отрицаний наружу или внесением внутрь, не задумываюсь о смысле выражения, а просто механически применяю формулы. Ошибок не возникает.
          +1
          Видимо потому что PVS-studio чаще всего C\C++ проекты проверяет открытые. А так да, проблема с операторами думаю для всех языков актуальна.
          +5
          По поводу тернарного оператора — уже давно вошло в привычку брать его в скобки. И условие в свои отдельные скобки. ИМХО — это должно войти в привычку у многих. Иначе от граблей из-за различных приоритетов долго будут страдать все…
          И еще — когда идут сложные условия ветвления (больше 1), рекомендуется хоть как-то составить таблицу истинности — никто не застрахован от ошибок, а так за счет пары лишних минут можно прилично уменьшить вероятность создания бага.
            +1
            Я вообще даже если можно и без скобок/кавычек все равно их ставлю. Это «можно» вроде как к естественному языку должно приближать, но по мне так и вероятность ошибок повышает.
              +6
              if( (pSymDef->GetType() != SbxEMPTY) ||
                  (pSymDef->GetType() != SbxNULL) )
              

              Наличие скобок в таких примерах не сильно помогает не допустить ошибку. С приоритетом операций в этом примере тоже всё понятно. Я хотел сделать акцент именно на необычные комбинации трёх операторов. Нужно просто запомнить, что такие ситуации бывают.
                0
                Жаль, что не было акцента на двух правильных вариантах. Ведь если условное выражение не совпадает с одним из этих вариантов, то значит оно неверно составлено.
                  0
                  Вы правильно сказали, что если условное выражение совпадает с одним из этих вариантов, то, скорее всего, оно составлено неверно.

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

                  Например, в условии ниже есть ошибка — оно всегда истинно:
                  if ( err != code1 || err != code2)
                  {
                    ....
                  }
                  

                  Сделав замену оператора || на && мы получим такое условие:
                  if ( err != code1 && err != code2)
                  {
                    ....
                  }
                  

                  которое будет истинно для любых значений err, не равных code1 и code2. В этом синтетическом примере отсутствует исходная логическая ошибка, но мы не можем утверждать, что это будет правильной правкой в каждом конкретном проекте. А условия, которые не зависят от части логического подвыражения, непонятно даже как исправить на синтетическом примере, не то, что в реальном коде.

                    0
                    Я несколько о другом говорил. Не знаю, как у всех, но части людей нельзя говорить про то, как делать нельзя, но не говорить про то, как делать можно (хотя оно логически вытекает из запрета). Простой пример: мать посылает сына в магазин за хлебом и говорит: «Купи любой, только батон нарезной не покупай». И пока он собирается и одевается, постоянно напоминает — «не покупай нарезной». Приходит сын в магазин и вспоминает — «Какой хлеб купить надо было? А! Мама постоянно про батон нарезной говорила, куплю его.»

                    Я не рассуждаю о том, что два оставшихся варианта всегда правильны. Но статья нам доказывает, что перечисленные четыре — всегда избыточны в количестве условий. Поэтому, если наше условное выражение не подходит под != && != или == || ==, то оно заведомо содержит в себе лишние условия.
                    Кому как, но мне проще запомнить два верных в большинстве случаев выражения, чем запоминать четыре, которые содержат лишнее условие.
                      +1
                      Существуют и такие, и такие методологии обучения, у всего есть плюсы/минусы, свои требования к аудитории. Я вот увидел статью в таком ключе.
                  0
                  Мы немного о разном. Ошибка допускается во время проигрыша операции в голове, скобки не помогут. Человек так производит операцию. Я говорил об этом, его так учили.
              –3
              Errare humanum est. Кстати, ошибки в логических выражениях внесены еще в ВУЗе прямо в головы разработчиков. Как и многие другие.
              Есть еще один хитрый источник — особенности самого процесса, не вдаваясь в физиологию ВНД скажу, что процесс идет с очень непостоянной скоростью. Естественно, концентрация тоже разная. Но это вопрос очень темный.
                +5
                Не понял пассажа про ошибки и ВУЗ, поясните, пожалуйста.
                  –6
                  Не обязательно ВУЗ. Но, классически одна из задач ВУЗа выработать навыки мышления и способность к обучению (я именно такой заканчивал), а не просто дать знания (бесполезные к окончанию). То есть если человек системно ошибается, навыки у него не выработаны, просто я видел как массово ошибались выпускники одного ВУЗа, что то не то было в обучении (известно что, но там вопрос религии). Интереснее второй момент. Людям свойственно ошибаться. ВУЗ не при чем, чистая психология

                  >--------<
                  <-------->

                  Нарисуйте это на бумаге сплошными линиями, очевидно, линии одной длинны, но на рисунке так не покажется! Значки меняют субъективное восприятие. Так же, полагаю, есть ситуации, когда человек ошибется с большой долей вероятности за счет фокуса со стрелками. Картина в мозге — не объективная реальность, а восприятие конкретным индивидуумом. А ВУЗ… ну люблю некоторых обучателей чихвостить, им в голову не приходит, что 18-летний все принимает на веру и для него это будет истина до конца жизни на уровне подсознания.
                    +5
                    Эмм, а что это за поток сознания? Где кому какие ошибки в вузе внушают\развивают?
                      –1
                      Профессиональные.
                        0
                        Это прям какой-то конкурс на звание «мистер-неопределенность».
                        Можно больше конкретики?
                          0
                          Вы не обратили внимание, «Как ошибаются профессионалы» — заголовок, но кто-то определенно сказал почему?
                          Если интересно — мне есть смысл пытаться что-то объяснить, нет, скучно смотреть на ведро минусов.
                          Я готов услышать, что неверного в утверждении «Errare humanum est».
                            0
                            Как в вашу теорию вписываются зарубежные разработчики, участвующие и делающие ошибки в открытых проектах? В западных вузах совсем иной подход к обучению программированию.
                              0
                              Может мое суждение «профессиональные» — ошибочно, готов признать. Но Цицерон врядли ошибся, я его я и цитировал, только часть фразы, вторая заставила бы задуматься.

                              Коротко, система, ищущая ошибки полезна, но использует метод, которым они создаются, или я ошибаюсь?
                0
                Осталось узнать, как оно работает при таком коде.
                  +1
                  Defensive programming. То, что не отловится одной проверкой заметит другая… скорее всего.

                  С одной стороны даёт возможность создавать современных монстров на сотни миллионов строк кода, а с другой — гарантирует что дыры в безопасности будут «лататься» вечно.

                  Чтобы дыр не было нужно писать fail-fast/crash-only software и/или использовать GIGO (тогда проверок в системе остаётся мало и каждую из них можно хорошо обдумать), но это замедляет разработку кода, потому так никто не делает…
                  0
                  Надо бы ещё добавить проверку флагов. Тоже большой источник ошибок.
                    +8
                    «Подстраховать себя от таких ошибок можно путём проверки своего кода с помощью построения таблиц истинности.»

                    Есть еще один метод — выучить, наконец, законы де Моргана.
                      +15
                      Ага, с их помощью не очень очевидное
                      err != code1 || err != code2

                      становится более понятным
                      !(err == code1 && err == code2)
                      +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 = code3
                        +3
                        Это вы к чему написали? В вашем случае ошибки нет.
                          0
                          См. https://habrahabr.ru/company/pvs-studio/blog/281316/#comment_8848274
                          –2
                          Т.е. в таких ситуациях писать switch безопаснее. Но длиннее…
                          Ещё можно было напомнить, что, когда компилятор не вычисляет правую часть выражения, при определённых условиях в || и &&
                          0
                          В случае, если заменить условие

                          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

                            –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

                              0
                              Это всё понятно. Только мало кто во время работы пишет экспериментальные программки.
                                +1
                                На каждом шагу. Ну, для проверки двух условий в ифе — нет, конечно, но перед тем, как притянуть в проект какой-то паттерн или либу — почти каждый раз.
                              0
                              А еще наверное частая ошибка когда без скобочек используют маски и сравнения вроде if ( a & 3 == b & 3), нет?
                                0
                                Я повидал много частых ошибок) Возможно, про флаги и маски можно будет свою историю рассказать, если поизучать всю нашу базу ошибок.
                                +1
                                А эта проверка только для простых типов проводится, или для всех? А то с C++-ной возможностью перегрузить операторы можно какое угодно поведение получить, и описанные случаи окажутся вполне корректными. Например, если делается интерпретатор/интероп для некоего слаботипизированного языка программирования, где "" == 0 && "" == false – true. Или eDSL с компактным синтаксисом, вроде boost::spirit, где старые операторы наделяют другой семантикой из-за невозможности добавлять новые.
                                  0
                                  Анализатор ищет неправильные конструкции, какие были приведены в примерах. Но как и во всех диагностиках, для правил V547 и V590 реализован ряд своих исключений, которые позволяют избавить диагностику от ложных срабатываний. Я сейчас не вспомню все исключения, тем более для двух правил. Скорее всего, благодаря им я не встречал ложные срабатывания в описанных вами ситуациях, зато встречал много ошибок на простом коде.
                                    +1

                                    Что также может быть интересно — для пользовательских операторов не работает short-circuit-поведение.

                                    0
                                    Если речь идет не о элементарных типах, а о классах, оператор сравнения для которых может быть перегружен, то конструкция
                                    if (en_RenderType==RT_BRUSH &&
                                    en_RenderType==RT_FIELDBRUSH)

                                    Может быть не ложной.
                                      +3
                                      Есть много способов выстрелить себе в ногу. Есть целые языки, устроенные так, чтобы люди, их использующие страдали до тех пор, пока не наберутся опыта и не перейдут на что-то менее ужасное. Но… Зачем? Равенство должно быть отношением эквиватентности, незачем из него делать невесть что.
                                        +2
                                        Лучше сразу пристрелите автора, чтоб не мучился и не мучил других.
                                      • UFO just landed and posted this here
                                          0
                                          понятно, что в
                                          if (err == ERR_OK || err != ERR_PENDING)…
                                          можно спокойно убрать кусок с ERR_OK, но КМК иногда это может быть «документацией», что обрабатываем, например, «хороший» и «плохие» сценарии, а «злой», в смысле асинхронный, будет, например, где-то ещё.
                                          Что в общем не должно мешать оторвать руки человеку, придумавшему такой API. 8-)
                                            0
                                            3 и 4 это ошибки. 1 и 2 нет.
                                            != || !=
                                            == || !=
                                            Оба случая — может достаточно наличия/отсутствия одного из двух признаков. Не согласен совершенно с автором статьи.
                                              +1
                                              Ну прям совершенно не согласны?) Это же реальные баги и реально ничего не делающий код.
                                              Оба случая — может достаточно наличия/отсутствия одного из двух признаков.

                                              Это как раз причина того, почему такие ошибки долго не замечают.

                                            Only users with full accounts can post comments. Log in, please.