• Hello Logify, или мониторим ошибки на установленных приложениях
    +5
    Разработчики PVS-Studio одобряют этот вопрос.
  • Hello Logify, или мониторим ошибки на установленных приложениях
    +6
    А что по поводу V3083? Например, здесь:
    public bool CopyHandler(int bytesCopied) {
        ....
        if (NotifyProgress != null)
            NotifyProgress(this, EventArgs.Empty);
        return !isStopped;
    }

    V3083 Unsafe invocation of event 'NotifyProgress', NullReferenceException is possible. Consider assigning event to a local variable before invoking it. Zip.cs 1035

    Или:
    protected override bool
    RaiseConfirmationDialogShowing(ReportConfirmationModel model) {
      if(ConfirmationDialogShowing != null) {
        ConfirmationDialogModel actualModel = model as ConfirmationDialogModel;
        if(actualModel == null)
            return false;
        ConfirmationDialogEventArgs args = new ConfirmationDialogEventArgs(actualModel);
        ConfirmationDialogShowing(this, args);
        return args.Handled;
      }
      return false;
    }

    V3083 Unsafe invocation of event, NullReferenceException is possible. Consider assigning event to a local variable before invoking it. LogifyClient.cs 169
    Здесь больше шанс возникновения NullReferenceException, так как проверка и использование разнесены дальше.

    Далее
    • V3083 Unsafe invocation of event, NullReferenceException is possible. Consider assigning event to a local variable before invoking it. LogifyClient.cs 156
    • V3083 Unsafe invocation of event 'FocusChanged', NullReferenceException is possible. Consider assigning event to a local variable before invoking it. BreadcrumbsRecorder.cs 391
    • V3083 Unsafe invocation of event 'PropertyChanged', NullReferenceException is possible. Consider assigning event to a local variable before invoking it. ReportConfirmationModel.cs 55
    • V3083 Unsafe invocation of event, NullReferenceException is possible. Consider assigning event to a local variable before invoking it. LogifyClientBase.cs 171
    • V3083 Unsafe invocation of event, NullReferenceException is possible. Consider assigning event to a local variable before invoking it. LogifyClientBase.cs 183
    • V3083 Unsafe invocation of event, NullReferenceException is possible. Consider assigning event to a local variable before invoking it. LogifyClientBase.cs 192

    P.S. Я не из вредности, а чтобы наглядно демонстрировать, как цена исправления растёт.
  • Hello Logify, или мониторим ошибки на установленных приложениях
    +8
    <зануда on> Хочу напомнить всем присутствующим, что продуктивнее не разово что-то проверить и поправить, а использовать статически анализ регулярно. Тогда многие ошибки можно было выявить не в процессе внутреннего тестирования (и последующему фидбека от пользователей), а сразу в ходе написания кода. <зануда off>
  • Hello Logify, или мониторим ошибки на установленных приложениях
    +20
    Учишь, учишь PVS-Studio использовать. Так ведь нет… :)

    void KeyDown(object sender, KeyEventArgs e) {
      FrameworkElement source = sender as FrameworkElement;
      if(!IsActive || (sender == null))
          return;
      Dictionary<string, string> properties =
        CollectCommonProperties(source, e);
      LogKeyboard(properties, e, false,
                  CheckPasswordElement(e.OriginalSource as UIElement));
    }

    V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword. Check variables 'sender', 'source'. BreadcrumbsRecorder.cs 86

    Видимо следует проверять на null ссылку source, а не sender.

    Аналогично:
    • V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword. Check variables 'sender', 'source'. BreadcrumbsRecorder.cs 94
    • V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword. Check variables 'sender', 'source'. BreadcrumbsRecorder.cs 102
    • V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword. Check variables 'sender', 'source'. BreadcrumbsRecorder.cs 110
    • V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword. Check variables 'sender', 'source'. BreadcrumbsRecorder.cs 118
    • V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword. Check variables 'sender', 'source'. BreadcrumbsRecorder.cs 126



    bool ValidateConfiguration(SenderConfiguration config) {
      if (String.IsNullOrEmpty(config.Url)) {
          Console.WriteLine("ERROR: service Url not specified");
          return false;
      }
      if (String.IsNullOrEmpty(config.ApiKey)) {
          Console.WriteLine("ERROR: ApiKey not specified");
          return false;
      }
      if (String.IsNullOrEmpty(config.ApiKey)) {
          Console.WriteLine("ERROR: ReportFileName not specified");
          return false;
      }
      ....
    }

    V3021 There are two 'if' statements with identical conditional expressions. The first 'if' statement contains method return. This means that the second 'if' statement is senseless Program.cs 159

    Copy-Paste классический. Два раза проверяется config.ApiKey.

    protected override bool SendExceptionReportCore(....) {
      ....
      lock (typeof(OfflineDirectoryExceptionReportSender)) {
      ....
    }

    V3090 Unsafe locking on a type. All instances of a type will have the same 'Type' object. TempDirectoryExceptionReportSender.cs 33

    Плохо делать lock, используя тип. Подробности в документации. И таких lock-ов ещё 7 штук.

    И т.д.
  • О PVS-Studio в преддверии открытой конференции ИСП РАН им. В.П. Иванникова
    –4
    Странно, а ведь диссертацию вроде защитил? Если да, то по логике должен уметь =)
    Это было так давно. Это было 217 хабрастатей тому назад. :)
  • Время устранять ошибки в Open-Source проектах, конкурс
    0
    Сейчас я и один мой коллега проверили — файл скачивается. Попробуйте ещё раз.

    Если вновь не получится, быть может проблема на вашей стороне или эта какая-то более хитрая ситуация. В этом случае напишите нам в поддержку, попробуем совместно разобраться.
  • Андрей Карпов считает, что код проекта Manticore качественнее, чем код проекта Sphinx
    0
    Красивая ошибка. Нет, пока такую взаимосвязь видеть ещё не научили. Но работаем и в этом направлении.
  • Передаю привет разработчикам компании Yandex
  • Статические анализаторы кода на примере ClickHouse
    +3
    Спасибо за статью!
  • Обзор дефектов кода музыкального софта. Часть 3. Rosegarden
    0
    Я (и думаю не только я) ничего не понял.
  • Обзор дефектов кода музыкального софта. Часть 3. Rosegarden
    0
    image
    А что? Я готов к вызду, как аварийная служба. :)
  • Обзор дефектов кода музыкального софта. Часть 3. Rosegarden
    +2
    При чем здесь какие-то особенные ситуации и исключения… Создаётся объект с помощью оператора new. Этот указатель не кладётся в какой-то умный указатель, а хранится в самом обыкновенном указателе. Следовательно, перед выходом из функции следует вызвать для этого указатель оператор delete. Если этого не сделать, произойдёт утечка памяти. Попутно, если объект не удаляется, это может статья причиной утечки не только памяти, но и других ресурсов (например, не будет закрыт файл). Но это другая история. Утечка памяти будет в любом случае.

    Быть может, Вы путаете C++ с каким-то другим языком, где встроен сборщик мусора?
  • Обзор дефектов кода музыкального софта. Часть 3. Rosegarden
    0
    Указатель не на блок памяти.
    А на что тогда?

    Напишите, пожалуйста, название компании в которой Вы работаете. Мне кажется, этой компании срочно требуется анализатор кода PVS-Studio и я попробую связаться с руководством. Я представляю, что там напрограммировано… :)
  • Обзор дефектов кода музыкального софта. Часть 3. Rosegarden
    +1
    Unicorn facepalm
    image

    Я понимаю, это прозвучит высокомерно, но предлагаю прочитать книжку про язык Си++, прежде чем дискутировать.
  • Обзор дефектов кода музыкального софта. Часть 3. Rosegarden
    0
    А кто будет удалять объект, созданный с помощью оператора new?
    std::ifstream *testFile =
      new std::ifstream(filename.toLocal8Bit(),
      std::ios::in | std::ios::binary);
    

    Создали объект и положили указатель в обыкновенную переменную. Где вызов delete testFile?

    Результат — утечка.
  • Обзор дефектов кода музыкального софта. Часть 3. Rosegarden
    0
    Немножко потролю. :) Если в мире опенсорса и gcc всё хорошо, то откуда же все эти ошибки берутся? Неужели, например, разработчики Valgrind не умеют использовать предупреждения компилятора?
    static
    Bool doHelperCallWithArgsOnStack (....)
    {
      ....
       if (guard) {
          if (guard->tag == Iex_Const
              && guard->Iex.Const.con->tag == Ico_U1
              && guard->Iex.Const.con->Ico.U1 == True) {
             /* unconditional -- do nothing */
          } else {
             goto no_match; //ATC
             cc = iselCondCode( env, guard );
          }
       }
      ....
    }

    Подробнее.
  • Обзор дефектов кода музыкального софта. Часть 3. Rosegarden
    +5
    Что касается недсотижимого кода, то это распространенное явление после больших правок — иногда лень чистить, иногда нет времени. Однако кроме как расход лишней памяти на диске у разработчика никаких последствий не несет — компилятор все это просекает, сообщает, если предупреждения не отключены, а код не генерирует в любом случае.

    Время восхитительных историй о безопасности кода
    image

    А как же тогда быть, например, с уязвимостью CVE-2014-1266 в iOS? Чуть подробнее см. здесь.
  • Как проверить, находится ли значение указателя в заданной области памяти
    0
    Не уверен, что получился удачный перевод: мало голосов и тихо. Поэтому для оживления приглашаю посмотреть запись нашего нового доклада: C++ CoreHard Autumn 2017, Поиск уязвимостей с использованием статического анализа кода (доклад с этой статьей никак не связан).

    И заодно вопрос: Сделать какую-то диагностику в PVS-Studio на тему статьи это безумие, да?
  • Любите статический анализ кода
    +1
    Попробовали на проекте в полторы тысячи строк.
    Для проекта в полторы тысячи строк плотность ошибок составляет 0-25 ошибок на 1000 строк. Так что ставить эксперименты на столь малых проектах имеет мало практического смысла. В них вообще может не быть ошибок.

    Так что pvs полезен, но безумно дорог,
    Это заблуждение, которое продолжает тиражироваться. Компании приходят и приобретают инструмент и только некоторые обсуждают скидки. Есть конечно те, кто говорит, что дорого. Но им просто не нужен инструмент и им был бы дорог и CppCat за 250$. Поэтому про это нет смысла говорить.

    PVS-Studio в рассчёте на 1 программиста дешевле, чем оплата аренды за квадратные метры в офисе, которые занимает программист. Если взять общие расходы на содержание программиста, то стоимость PVS-Studio вообще не заметна на их фоне.

  • Философия статического анализа кода: у нас 100 программистов, анализатор нашел мало ошибок, он бесполезен?
    0
    Ну, когда-нибудь можно будет. :)
  • Философия статического анализа кода: у нас 100 программистов, анализатор нашел мало ошибок, он бесполезен?
    –1
    Давайте лучше это считать преувеличением, которое призвано выделить главную мысль. Мир вообще сложный и про него трудно говорить. Поэтому придумывают упрощенные модели, чтобы разобраться с каким-то вопросом и пояснить другим. :)
  • Философия статического анализа кода: у нас 100 программистов, анализатор нашел мало ошибок, он бесполезен?
    0
    Да только проблема не в том, что анализатор не найдет ошибок, а в том что выдаст миллион ложных срабатываний.
    Вы преувеличиваете размер беды. Да, поначалу анализаторы кода могут выдавать огромное количество предупреждений. Однако, все серьёзные анализаторы кода предоставляют различные варианты настройки. И уже после минимальной настройки, количество ложных срабатываний может быть уменьшено в разы. Больше всего ложных срабатываний относится к неудачным макросам. Разметив эти макросы, можно получить сразу намного более приятную картину в отчёте.

    Например, вот статья где идёт речь о количестве ложных срабатываний, выдаваемых PVS-Studio: Характеристики анализатора PVS-Studio на примере EFL Core Libraries, 10-15% ложных срабатываний.

    Единственный вариант использовать его с самых первых строк нового проекта.
    Да, это вполне вариант. Но почти нереальный на практике. :)

    Поэтому в PVS-Studio есть вариант разметить все предупреждения как пока неинтересные и работать только с предупреждениями, относящимися к новому или изменённому коду. Это позволяет легко внедрить использование статического анализа в большой проект и начать сразу получать от него пользу. Подробнее: Массовое подавление сообщений анализатора (отключение выдачи предупреждений на существующий код).
  • Любите статический анализ кода
    +1
    Я не понимаю вопрос. Если что-то компилируется в Windows и Linux, Вы можете это проверить, используя мониторинг компиляции (см. документацию). Если у Вас какой-то особый сложный случай, то прошу прийти в поддержку и мы пообщаемся.
  • Любите статический анализ кода
    +1
    На данный момент возможна интеграция PVS-Studio с CLion, QtCreator, SonarQube и т.д. Подобнее.
  • Любите статический анализ кода
    0
    Такая возможность давно существует в PVS-Studio. Есть несколько способов проверки проектов. Проще всего начать с системы мониторинга компиляции: windows, linux (см. «Любой проект»).
  • Философия статического анализа кода: у нас 100 программистов, анализатор нашел мало ошибок, он бесполезен?
    +5
    Хочу заметить, что такой анализатор не спасет от логических ошибок в бизнес-логике приложения. Тут помогут только юнит тесты.
    Про бизнес логику Вы правы.

    Но эти же самые тесты выявят любые ошибки, в том числе и те, что обнаруживает статический анализатор.
    А вот здесь дело обстоит иначе. Теоретически утверждение верно, но к практике оно не применимо.
    Статические анализаторы и юнит-тесты не конкурируют, а дополняют другую. У юнит-тестов есть слабые места, которые отлично подстрахует статический анализ:

    • Сами юнит-тесты часто содержат ошибку. Для юнит-тестов не пишут другие юнит-тесты. :) Зато анализатор проверяет тесты. И по моему опыту, там очень часто находятся ошибки. Чаще всего выясняется, что тест ничего не тестирует.
    • Не все ошибки можно найти юнит-тестами. Вернее, это слишком сложно. Например, юнит тестом очень сложно найти ошибку обнуления стековой памяти при выходе из функции. Я имею в виду вот это. Или, например, юнит-тестом можно не найти чтение из неинициализированной переменной (тестам повезёт и там будет хорошее значение).
    • Слишком дорого покрыть тестами весь код. Статические анализаторы проверяют весь код. В результате, находится огромное количество ошибок в коде, который редко выполняется.
    • Непонятно что делать с большим проектом, в котором мало юнит-тестов, а хочется в обозримом времени улучшить его качество. Писать тесты — слишком долго и дорого. Статический анализ в этот момент становится хорошей альтернативой.


  • Философия статического анализа кода: у нас 100 программистов, анализатор нашел мало ошибок, он бесполезен?
    0
    Непонятная какая-то аналогия.
  • Любите статический анализ кода
    0
    1) Я уже писал выше, что функция LoadString размечена «не на полную». Сложно аннотируя тысячи функций сразу учесть все способы их неправильного использования. Доработаем. Собственно, во многом благодаря подомным рекомендациям пользователей анализатор улучшает свои возможности.

    2) Кажется это было сделано специально. Уж очень никто не проверяет результат LoadString. :) Я изучу этот вопрос.
  • Любите статический анализ кода
    0
    Объявите функцию в cpp файле где она используется. Так себе решение, но хоть ошибка исправится.
  • Любите статический анализ кода
    –1
    А теперь попробуйте PVS-Studio. :)
  • Любите статический анализ кода
    0
    Значит по делу ругается.
  • Любите статический анализ кода
    0
    Не понятно, это похвала анализатора или критика за ложное срабатывание? :) Прошу уточнить.

    Проверьте, есть ли объявление функции ReadPictureName в texman.c. Скорее всего нет, а раз так, считается, что функция возвращает int. Как следствие, имеем красивую 64-битную ошибку.
  • Любите статический анализ кода
    0
    Прозвучало (не здесь, а в другом месте), что второй пример выдуманный. Нет, описанные случаи в статье взяты из реальных проектов. Я не стал писать названия, так как не посчитал, что это важно. Источник: definesTypes.h
  • Любите статический анализ кода
    +6
    Посмотрел, как у нас размечена функция LoadString. К сожалению, предупреждения не будет. Доработаем.
  • Любите статический анализ кода
    0
    Кстати, Visual C++ борется с некоторыми подобными случаями (в том числе и с приведённым define). Я писал про это заметку 5 лет назад — Прощай #define private public.
  • Любите статический анализ кода
    +2
    Скорее всего, программа с "#define true false" просто не скомпилируется. Или программа станет столь неработоспособна, что невозможно будет не обратить внимание на это.

    В любом случае, "#define true false" это какое-то вредительство и не стоит о нём специально думать. Если уж кто-то захочет навредить, он придумает как это сделать, чтобы это было и не заметно и скрылось от анализаторов.

    На практике больший интерес представляют случаи неумышленно неправильного написания #define. Например, могут быть забыты скобки. Мы уже выявляем некоторые такие ситуации (см., например, V1003) и будем развивать анализатор в этом направлении и дальше.
  • Любите статический анализ кода
  • Начальник, что мне делать для того, чтобы получать больше денег
    +3
    Если честно, я тут со всеми не согласен. Обсуждается какой-то черно белый мир с разных точек зрения, к которым спорящие прибиты гвоздиком :). Мир сложнее, многоцветнее, а точки зрения могут дрейфовать в зависимости от условий и решаемых бизнесом задач.
  • Начальник, что мне делать для того, чтобы получать больше денег
    +2
    Это личное мнение человека, который сейчас трудится в другой компании.
  • Философия статического анализа кода: три простых шага
    0
    Это не противоречие, а невозможность указать один единственный верный способ использования анализатора кода.

    Отключать отдельные диагностики можно. Можно исключать определённые папки с каталогами. И так далее. Подробности описаны в документации. Здесь же была сделана дать некое общее видение ситуации.