Повторная проверка TortoiseSVN с помощью анализатора кода PVS-Studio

    TortoiseSVN и PVS-Studio
    Мы отправили разработчикам TortoiseSVN на некоторое время бесплатный ключ для анализатора PVS-Studio. Пока они не успели им воспользоваться, я решил быстро скачать исходные коды TortoiseSVN и самостоятельно выполнить анализ. Цель понятна. Очередная небольшая статья для рекламы PVS-Studio.

    Мы уже проверяли проект TortoiseSVN. Это было давно. Проверка проекта как раз совпала с выпуском PVS-Studio 4.00, где впервые появились диагностические правила общего назначения.

    Время от времени мы повторяем проверки, чтобы продемонстрировать пользу от регулярного использования инструмента. Нет смысла один или два раза проверить проект. В изменяемом коде постоянно появляются новые ошибки. А потом медленно и печально исправляются. Соответственно, максимальная польза будет достигнута при ежедневном использовании PVS-Studio. А ещё лучше, при использовании инкрементального анализа.

    Итак, посмотрим, что удалось найти интересного в проекте с помощью PVS-Studio версии 5.05. Исходные коды TortoiseSVN были взяты 19.06.2013 из http://tortoisesvn.googlecode.com/svn/trunk. Кстати, проект TortoiseSVN является очень качественным и имеет огромную базу пользователей-программистов. Так что если найти хоть что-то, это уже большое достижение.

    Странные условия


    static void ColouriseA68kDoc (....)
    {
      if (((sc.state == SCE_A68K_NUMBER_DEC) && isdigit(sc.ch))
          ....
          || ((sc.state == SCE_A68K_MACRO_ARG) && isdigit(sc.ch))
          || ((sc.state == SCE_A68K_MACRO_ARG) && isdigit(sc.ch))
          ....
    }

    Диагностическое сообщение: V501 There are identical sub-expressions '((sc.state == 11) && isdigit(sc.ch))' to the left and to the right of the '||' operator. lexa68k.cxx 160

    Присутствует два одинаковых сравнения. Возможно, имеет место опечатка.

    Опечатка, наверное, присутствует и в следующем коде. Два раза проверяется значение переменной 'rv'.
    struct hentry * AffixMgr::compound_check(
      ....
      if (rv && forceucase && (rv) && ....)
      ....
    }

    Диагностическое сообщение: V501 There are identical sub-expressions to the left and to the right of the '&&' operator: rv && forceucase && (rv):
    • affixmgr.cxx 1784
    • affixmgr.cxx 1879

    Следующий фрагмент кода:
    bool IsAllASCII7 (const CString& s)
    {
      for (int i = 0, count = s.GetLength(); i < count; ++i)
        if (s[i] >= 0x80)
          return false;
        return true;
    }

    Диагностическое сообщение: V547 Expression 's[i] >= 0x80' is always false. The value range of char type: [-128, 127]. logdlgfilter.cpp 34

    Функция IsAllASCII7() всегда возвращает 'true'. Условие 's[i] >= 0x80' всегда ложно. Значение переменной типа 'char' не может быть больше или равно 0x80.

    Следующий фрагмент кода с неправильным сравнением:
    int main(int argc, char **argv)
    {
      ....
      DWORD ticks;
      ....
      if (run_timers(now, &next)) {
        ticks = next - GETTICKCOUNT();
        if (ticks < 0) ticks = 0;
      } else {
        ticks = INFINITE;
      }
      ....
    }

    Диагностическое сообщение: V547 Expression 'ticks < 0' is always false. Unsigned type value is never < 0. winplink.c 635

    Переменная 'ticks' является беззнаковой. Это значит, что проверка «if (ticks < 0)» не имеет смысла. Ситуация возникновения переполнения не будет обработана.

    Рассмотрим ошибку, из-за которой функция 'strncmp' сравнивает строки не полностью.
    int  AffixMgr::parse_convtable(...., const char * keyword)
    {
      char * piece;
      ....
      if (strncmp(piece, keyword, sizeof(keyword)) != 0) {
      ....
    }

    Диагностическое сообщение: V579 The strncmp function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. affixmgr.cxx 3654

    Оператор 'sizeof' вычисляет размер указателя. Это значение никак не связанно с длиной строки.

    Подозрительное формирование строки


    Функции с переменным количеством аргументов всегда везде есть, и как всегда опасны.
    class CTSVNPath
    {
      ....
    private:
      mutable CString m_sBackslashPath;
      mutable CString m_sLongBackslashPath;
      mutable CString m_sFwdslashPath;
      ....
    };
    
    const FileStatusCacheEntry * SVNFolderStatus::BuildCache(
      const CTSVNPath& filepath, ....)
    {
      ....
      CTraceToOutputDebugString::Instance() (_T(__FUNCTION__)
        _T(": building cache for %s\n"), filepath);
      ....
    }

    Диагностическое сообщение: V510 The 'operator()' function is not expected to receive class-type variable as second actual argument:
    • svnfolderstatus.cpp 150
    • svnfolderstatus.cpp 355
    • svnfolderstatus.cpp 360

    Спецификатор "%s" указывает, что в качестве фактического аргумента функция ожидает строку. Однако, переменная 'filepath' вовсе не строка, а сложный объект, состоящий из множества строк. Затрудняюсь сказать, что будет распечатано и не упадёт ли вообще этот код.

    Опасно использовать такие функции, как 'printf()' следующим образом: «printf(myStr);». Если внутри 'myStr' будут присутствовать управляющие спецификаторы, то программа может распечатать то, что ей не полагается или аварийно завершиться.

    Рассмотрим код из TortoiseSVN:
    BOOL CPOFile::ParseFile(....)
    {
      ....
      printf(File.getloc().name().c_str());
      ....
    }

    Диагностическое сообщение: V618 It's dangerous to call the 'printf' function in such a manner, as the line being passed could contain format specification. The example of the safe code: printf("%s", str); pofile.cpp 158

    Если имя файла будет «myfile%s%i%s.txt», то результат будет плачевен.

    Примечание. У нас есть интересная заметка на тему опасности использования функции printf().

    Неправильное обнуление массивов


    Я не знаю, насколько для TortoiseSVN опасно оставить содержимое буферов, не обнулив их. Возможно, вообще безопасно. Однако есть код для обнуления буферов. А поскольку он не работает, стоит про это упомянуть. Ошибки выглядят так:
    static void sha_mpint(SHA_State * s, Bignum b)
    {
      unsigned char lenbuf[4];
      ....
      memset(lenbuf, 0, sizeof(lenbuf));
    }

    Диагностическое сообщение: V597 The compiler could delete the 'memset' function call, which is used to flush 'lenbuf' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. sshdss.c 23

    Перед выходом из функции, массив 'lenbuf' следует очистить. Так как затем массив больше не используется, оптимизатор удалит вызов функции 'memset'. Чтобы этого не происходило, требуется использовать специальные функции.

    Другие места, где компилятор удалит вызов 'memset()':
    • sshdss.c 37
    • sshdss.c 587
    • sshdes.c 861
    • sshdes.c 874
    • sshdes.c 890
    • sshdes.c 906
    • sshmd5.c 252
    • sshrsa.c 113
    • sshpubk.c 153
    • sshpubk.c 361
    • sshpubk.c 1121
    • sshsha.c 256

    Странное


    BOOL InitInstance(HINSTANCE hResource, int nCmdShow)
    {
      ....
      app.hwndTT; // handle to the ToolTip control
      ....
    }

    Диагностическое сообщение: V607 Ownerless expression 'app.hwndTT'. tortoiseblame.cpp 1782

    Скорее всего, в функции 'InitInstance()', член 'hwndTT' должен чем-то инициализироваться. Однако, из-за опечатки, код оказался недописанным.

    64-битные ошибки


    Поиск ошибок я делаю весьма поверхностно. Я внимателен, ровно на столько, чтобы мне хватило примеров для написания статьи. Нет, я не бяка. Просто авторы проекта, всё равно выполнят анализ качественнее, чем могу сделать я.

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

    Приведу только пару опасных мест:
    void LoginDialog::CreateModule(void)
    {
      ....
      DialogBoxParam(g_hmodThisDll, MAKEINTRESOURCE(IDD_LOGIN),
                     g_hwndMain, (DLGPROC)(LoginDialogProc),
                     (long)this);
      ....
    }

    Диагностическое сообщение: V220 Suspicious sequence of types castings: memsize -> 32-bit integer -> memsize. The value being casted: 'this'. logindialog.cpp 105

    Указатель 'this' явно приводится к типу 'long'. Затем он неявно расширяется до типа LPARAM (LONG_PTR). Важно то, что указатель на какое-то время превращается в 'long'. Это плохо, если программа является 64-битной. Указатель занимает 64-бита. Тип 'long' в Win64 по прежнему является 32-битным типом. В результате старшие биты 64-биной переменной будут потеряны.

    Если объект создан за пределами младших 4 Гб оперативной памяти, то работа программы станет непредсказуема. Вероятность такого события конечно не велика, но зато такую ошибку крайне сложно воспроизвести.

    Правильный код: DialogBoxParam(...., (LPARAM)this);

    Рассмотрим другое опасное приведение типа:
    static int cmpforsearch(void *av, void *bv)
    {
      Actual_Socket b = (Actual_Socket) bv;
      unsigned long as = (unsigned long) av,
                    bs = (unsigned long) b->s;
      if (as < bs)
        return -1;
      if (as > bs)
        return +1;
      return 0;
    }

    Диагностическое сообщение: V205 Explicit conversion of pointer type to 32-bit integer type: (unsigned long) av:
    • winnet.c 139
    • winhandl.c 359
    • winhandl.c 348

    Указатели явным образом приводятся к типу 'unsigned long' и помещаются в переменные 'as' и 'bs'. Так как старшие биты адреса при этом могут быть потеряны, сравнение может работать неправильно. Вообще не понятно, зачем здесь указатели приводятся к целочисленным типам. Можно просто сравнить указатели.

    Устаревшие проверки нулевых указателей


    Оператор 'new', когда не может выделить память, давным-давно не возвращает NULL. Он генерирует исключение std::bad_alloc. Конечно, можно сделать, чтобы оператор 'new' возвращал 0, но это сейчас к делу не относится.

    Тем не менее, в программах продолжает жить код следующего вида:
    int _tmain(....)
    {
      ....
      pBuf = new char[maxlength];
      if (pBuf == NULL)
      {
        _tprintf(_T("Could not allocate enough memory!\n"));
        delete [] wc;
        delete [] dst;
        delete [] src;
        return ERR_ALLOC;
      }
      ....
    }

    Диагностическое сообщение: V668 There is no sense in testing the 'pBuf' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error.
    • subwcrev.cpp 912
    • repositorybrowser.cpp 2565
    • repositorybrowser.cpp 4225
    • svnstatuslistctrl.cpp 5254
    • svnprogressdlg.cpp 2357
    • bugtraqassociations.cpp 116
    • xmessagebox.cpp 792
    • xmessagebox.cpp 797
    • hyperlink_base.cpp 166
    • affixmgr.cxx 272
    • hashmgr.cxx 363
    • hashmgr.cxx 611

    И так сойдёт


    Про многие ошибки, с которыми я сталкиваюсь, изучая код, я не пишу в статьях. Дело в том, что они не мешают работе программы. В этот раз я решил написать о паре таких случаев. Просто очень забавно наблюдать ситуации, когда программа работает не от того, что она правильно написана, а из-за везения.
    void CBaseView::OnContextMenu(CPoint point, DiffStates state)
    {
      ....
      popup.AppendMenu(MF_STRING | oWhites.HasTrailWhiteChars ?
                       MF_ENABLED : (MF_DISABLED|MF_GRAYED),
                       POPUPCOMMAND_REMOVETRAILWHITES, temp);
      ....
    }

    Диагностическое сообщение: V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '|' operator. baseview.cpp 2246

    В зависимости от значения переменной 'oWhites.HasTrailWhiteChars' требуется получить одно из сочетаний констант:
    • MF_STRING | MF_ENABLED
    • MF_STRING | MF_DISABLED | MF_GRAYED

    Но код работает совсем не так. Приоритет операции '|' выше, чем приоритет операции '?:'. Расставим скобки для наглядности:

    (MF_STRING | oWhites.HasTrailWhiteChars)? MF_ENABLED: MF_DISABLED | MF_GRAYED

    Код корректно работает, только благодаря тому, что константа 'MF_STRING' равна 0. Она не оказывает никакого влияния на результат. В результате, неправильное выражение работает правильно.

    Рассмотрим другой пример везения. В программе TortoiseSVN тип HWND нередко используется как тип 'unsigned'. Для этого приходится выполняться явные преобразования типа. Например, так, как показано в следующих функциях:
    HWND m_hWnd;
    UINT_PTR uId;
    INT_PTR CBaseView::OnToolHitTest(....) const
    {
      ....
      pTI->uId = (UINT)m_hWnd;
      ....
    }
    
    UINT_PTR  idFrom;
    HWND m_hWnd;
    
    BOOL CBaseView::OnToolTipNotify(
      UINT, NMHDR *pNMHDR, LRESULT *pResult)
    {
      if (pNMHDR->idFrom != (UINT)m_hWnd)
        return FALSE;
      ....
    }

    Или, например, значение переменной типа HWND печатается, как если бы это был тип 'long'.
    bool CCommonAppUtils::RunTortoiseProc(....)
    {
      ....
      CString sCmdLine;
      sCmdLine.Format(L"%s /hwnd:%ld",
        (LPCTSTR)sCommandLine, AfxGetMainWnd()->GetSafeHwnd());
      ....
    }

    Формально этот код неверен. Дело в том, что тип 'HWND' представляет собой указатель. А значит, его нельзя превращать в 32-битные целочисленные типы. И анализатор PVS-Studio переживает по этому поводу, выдавая предупреждения.

    Но интересно то, что этот код будет работать совершенно корректно!

    Тип HWND используется для хранения дескрипторов, которые используются в Windows для работы с различными системными объектами. Такими же типами являются HANDLE, HMENU, HPALETTE, HBITMAP и так далее.

    Хотя дескрипторы являются 64-битными указателями, для большей совместимости (например, для возможности взаимодействия между 32-битынми и 64-битными процессами) в них используется только младшие 32-бита. Подробнее смотри "Microsoft Interface Definition Language (MIDL): 64-Bit Porting Guide" (USER and GDI handles are sign extended 32b values).

    Помещая тип HWND в 32-битные типы, разработчики вряд ли основывались именно на этих допущениях. Скорее всего, это не очень аккуратный код, работающий корректно благодаря везению и стараниям разработчиков Windows API.

    Вывод


    Используйте статический анализ регулярно при разработке, и вы найдете множество ошибок на самых ранних этапах. Я естественно рекомендую в первую очередь познакомиться с анализатором кода PVS-Studio. Однако, есть много других хороших анализаторов кода: инструменты статического анализа кода.

    Ссылки


    Дополнительные ссылки, которые могут пояснить некоторые тонкие моменты, описываемые в статье.
    1. База знаний. Перезаписывать память — зачем?
    2. Документация. V668. There is no sense in testing the pointer against null, as the memory was allocated using the 'new' operator.
    3. База знаний. Как корректно привести указатель к int в 64-битной программе?
    4. Карпов Андрей, Евгений Рыжков. Уроки разработки 64-битных приложений на языке Си/Си++.
    PVS-Studio
    Static Code Analysis for C, C++, C# and Java

    Comments 35

      +5
      >> Функция IsAllASCII7() всегда возвращает 'true'.…
      А вот это далеко не факт, у многих компиляторов (в том числе и Visual Studio) есть опции, которые делают типа char по умолчанию как unsigned char. Хоть это и костыльный механизм, но такое используется.
        +31
        Плагин PVS-Studio проверяет наличие этого ключа и передаёт информацию анализатору. В общем, этот момент учтён.
          +1
          Вот теперь понятно.
          +1
          Костыльный механизм?
          Вообще, под той же вижуалкой очень широко принято использовать ключ компиляции _UNICODE в настройках проекта, который заставляет компилироваться MFC'шный шаблон CString на базе wchar_t, который не просто unsigned char, а самый что ни на есть WORD.
          В контексте данной функции сия ремарка может быть вполне уместна. Просто, возможно, автор поста анализировал ANSI-версию (если, конечно, таковая до сих пор осталась в TortoiseSVN), либо анализатор не учитывает ключей компиляции, либо про них забыли сами разработчики.
          +8
          Раздел «И так сойдёт» порадовал, люблю такие ошибки.
            –1
            Ну работает же как то?
            • UFO just landed and posted this here
                0
                Откиньтесь на спинку кресла и отдохните, пока %driver_name% устанавливается на ваш компьютер. Во время установки вы узнаете о новых замечательных возможностях %driver_name% и о некоторых из наиболее важных функций системы. После завершения установки компьютер пригласит вас на более подробный курс ознакомления с %driver_name%.
              +3
              А как ваша программа справится с более сложным кодом, например с Boost? Может ли она находить ошибки/странности при использования шаблонов. К примеру что-то типа #8702:
              template<typename T>
              class has_size_type
              {
                  typedef char no_type;
                  struct yes_type { char dummy[2]; };
              
                  template<typename C>
                  static yes_type test(BOOST_DEDUCED_TYPENAME C::size_type x);
              
                  template<typename C, typename Arg>
                  static no_type test(Arg x); // Ошибка здесь. Должен быть '...' вместо шаблонного параметра
                  /*утилита может выдать предупреждение о том, что "ищется строгое соответствие с int. Вы имели в виду "..."?"*/
              
              public:
                  static const bool value = sizeof(test<T>(0)) == sizeof(yes_type);
              };
              
                +7
                Boost мы прожёвываем. Другое дело, что искать ошибки в шаблонах дело очень неблагодарное. Кое-что мы находим, но мало. Например, одна из бед в том, что шаблоны нередко содержат вообще не компилируемый код. Шаблоны приходится «аккуратно трогать палочкой». Ибо неизвестно, что там. Кстати, Visual C++ проглатывает некорректные неиспользуемые функции в шаблонах. При попытке их использовать возникнет ошибка компиляции. Далеко ходить не надо. Такие функции есть даже в заголовочных файлах от Visual C++. См. например файл «Microsoft Visual Studio 8\VC\atlmfc\include\atldb.h»:

                template <const GUID* pguidProvider>
                class CErrorReporterHelper
                public:
                  HRESULT PostError(HRESULT hrErr, IID* piid)
                  {
                    . . .
                    errorinfo.hrError  = hrErr;
                    errorinfo.iid    = *piid;
                    spCrtErrInfo->SetGUID(errorinfo.iid)
                    spCrtErrInfo->SetSource(OLESTR("Provider PROGID"));
                


                После SetGUID(errorinfo.iid) нет точки с запятой.

                Возвращаясь к приведенному примеру. Нет, не найдем. Слишком новые и тонкие материи. Мы ищем более приземленные вещи. И уверен, правильно, что поступаем так. На одну заковырку приходится с десяток обыкновенных опечаток и просто ляпов. Да, это именно так. :) Ощутите глубину глубин.
                  0
                  Ощутите глубину глубин.

                  Ощутил. Внушает.

                  А вы не хотели бы сделать как один из ваших конкурентов (кажется Coverity): выложить ошибки опенсорсных проектов чтобы разработчики могли их поправить. Я после ваших предыдущих постов искал отдельную ссылку ошибок по Boost, но не нашел :(
                    +3
                    Проверяя очередной проект, мы пишем статью. Уведомляем авторов проекта о найденных недочетах. Выдаем им на некоторое время анализатор, чтобы они смогли более тщательно проверить проект, чем делаем это мы. То, что находим мы сами, оформляется в виде базы ошибок. Этого вполне достаточно для демонстрации.

                    Сам Boost мы не проверяли. Там сложная система сборки, в которую непросто интегрироваться. По крайней мере, с наскока проверить Boost не удалось. А стимула заняться библиотекой более плотно у не было. Тот же Coverity проверят open source проекты не из-за любви к искусству, а за гранты от United States Department of Homeland Security (DHS).

                    Был ещё какой-то энтузиаст, который очень хотел проверить Boost. Утверждал, что сам сделает это. Мы выдали ему на время ключ. Он пропал. Видимо тоже не смог разобраться в системе сборки.

                    +1
                    Глубина, глубина, я не твой. (с)
                  0
                  Два вопроса:
                  1) Возможно ли как-то прикрутить PVS Studio к Эклипсу? Или, может быть, интеграция планируется в будущем?

                  2) Ругается ли PVS на такие вещи:

                  uint8_t size;
                  if(size >= UART_RX_MSG_SIZE_MIN && size <= UART_RX_MSG_SIZE_MAX) ( компилятор выдает ворнинг — comparison is always true, потому что сейчас эти дефайны равны 0 и 255. Но это не обязательно всегда так. Аналогичные ворнинги при сравнении с параметрами шаблонов)
                    0
                    Отвечу про Эклипс. Вот здесь описано как использовать анализатор из командной строки. Если такой вариант устроит (без плагина, без автоматической интеграции), то тогда использовать можно.
                      0
                      Для этого все равно нужен проектный файл VS, так что не очень.
                        0
                        Я ошибся и дал ссылку не туда. Вот правильная ссылка. Где как раз не нужен проектный файл.
                          0
                          О, да, спасибо.
                      +4
                      Пункт N2.

                      Да, PVS-Studio тоже предупредит. Как человеку, мне понятно, что тут всё хорошо. С точки зрения анализатора, не понятно на чем основывать исключение из правила. По отдельности, каждая проверка весьма подозрительна. И лучше перестраховаться и предупредить.
                      Ложное срабатывание не так страшно. В PVS-Studio есть масса методов их подавления. Например, можно использовать специальный комментарий:

                      if(size >= 0 && size <= 255) //-V547
                      


                      Ещё вариант — указать, что там где используется макросы UART_RX_MSG_SIZE_MIN и UART_RX_MSG_SIZE_MAX, не стоит выдавать диагностику V547. Для этого в заголовочном файле рядом с макросами можно вписать:

                      #define UART_RX_MSG_SIZE_MIN 0
                      #define UART_RX_MSG_SIZE_MAX 255
                      //-V:UART_RX_MSG_SIZE_MIN:547
                      //-V:UART_RX_MSG_SIZE_MAX:547
                      


                      Чтобы убедить, что диагностика V547 ой как полезна, предлагаю посмотреть примеры ошибок, выявляемые ей. Там встретится и сравнение с 0, и с 255.
                        0
                        Подавление комментарием — это способ понятный и хороший.

                        Но почему бы просто не давить предупреждение, если происходит сравнение с дефайном или параметром шаблона? По ссылке-то сравнения с магическими числами (если только вы дейфайны не убирали самостоятельно)
                          0
                          Макросы часто используются. Вполне может быть ошибка. То что именно такого примера нет, ни о чем не говорит. Зато, например, есть такое:
                          #define DISKREADBUFFER_SIZE HEX(10000)
                          
                          typedef unsigned short USHORT, *PUSHORT;
                          
                          static VOID DetectBiosDisks(....)
                          {
                            USHORT i;
                            ....
                            Changed = FALSE;
                            for (i = 0; ! Changed && i < DISKREADBUFFER_SIZE; i++)
                            {
                              Changed = ((PUCHAR)DISKREADBUFFER)[i] != 0xcd;
                            }
                            ....
                          }
                          
                          V547 Expression 'i < 0x10000' is always true. The value range of unsigned short type: [0, 65535]. xboxhw.c 358
                            0
                            Резонно.
                          +1
                          По пункту один позволю себе сформулировать пожелание:
                          было бы очень удобно иметь возможность из командной строки проверять директорию целиком, а не отдельные файлы. Cppcheck может, а проверять больше 10 файлов за раз в ручном режиме по одному — достаточно неудобно.
                            0
                            Батники писать разучились?
                              0
                              Нет, но PVS-Studio стала ругаться, что строка с файлами слишком большая :)
                              Плюс этот батник пришлось бы руками обновлять при добавлении/удалении файлов из проекта.
                                0
                                Мне кажется, Вы что-то не то пытаетесь делать. Командная версия предназначена для интеграции в систему сборки (вызов из makefile и т.п.). Идея в том, чтобы заменить вызов компилятора на вызов PVS-Studio, немного пошаманив. Не понятно, зачем пытаться придумывать командные строки для абстрактного набора файлов.

                                Напишите нам в поддержку. Попробуем разобраться с Вашей ситуацией.
                                  0
                                  Я же говорю — разучились.
                                    0
                                    Мне очень стыдно, но на самом деле я и не умел никогда.
                          +1
                          Функция IsAllASCII7() всегда возвращает 'true'. Условие 's[i] >= 0x80' всегда ложно. Значение переменной типа 'char' не может быть больше или равно 0x80.

                          Неправда: TortoiseSVN компилируется в unicode, а это значит что s[i] возвращает WCHAR который без проблем может быть больше 0x80.
                            0
                            Да, действительно. Что-то видимо пошло при анализе не так. Потом посмотрю. Вот и верь таким рекламщикам, как я :). В погоне за статьёй уже некогда всматриваться в код.
                            Кстати, именно поэтому я всегда настаиваю, чтобы авторы сами проверяли проекты. То что я считаю ошибкой, может оказаться ложным срабатыванием и наоборот. Лучше проверить самостоятельно, зная код. И нам написать про явные ляпы.
                              0
                              Ну вот, а я по наивности думал, что никто кроме меня эту тему не заметил =). Особенно с учётом что первый же комментарий к статье был на эту же тему.
                              +1
                              Я не представляю, как вы, ребята, вообще используете хоть какой-то софт, зная, что там внутри такие баги. Это всё равно что есть колбасу, зная, из чего она сделана.
                                0
                                Всё просто. Найденные ошибки содержатся в редко используемых фрагментах кода или вероятность их возникновения крайне мала (например, усечение 64-битного указателя). Более часто проявляющие себя ошибки находятся на этапе тестирования и пользователями. Что медленно, печально и дорого. Поэтому я всегда подчеркиваю:

                                Основная ценность инструментов статического анализа не в том, что они позволяет находить ошибки в коде редко получающего управления. А в нахождении ошибок на самом раннем этапе. Многие из ляпов, которые будут найдены тестированием или пользователями, можно было бы устранить программисту ещё на этапе кодирования.
                                0
                                А вы не пробовали по WRK так пройтись? Там, конечно, лицензия не позволяет публиковать куски кода, но все равно интересно.
                                  0
                                  Проверяли где-то полтора года назад. На тот момент нашлось всего одна ошибка и пара подозрительных мест. Но в любом случае на заметку не хватит. С тех пор не возвращались к этому проекту, так как непонятно, что дальше делать, даже если найдём ошибки.

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