28 December 2011

Статический анализатор кода PC-Lint

Designing and refactoring
Чтобы не откладывать дело в «долгий ящик», продолжу рассказ о своем опыте использования статических анализаторов кода. (Начало здесь и здесь).

Попробовав Klocwork, я попытался заинтересовать в нем руководство, однако цена в 30 тыс. евро послужила главным останавливающим критерием, все остальное было уже не важно.

Однако кто-то вдруг вспомнил, что однажды компания уже приобретала лицензию на какой-то анализатор кода. Этим анализатором оказался PC-Lint. Оказалось, что его никто не использует (дочитав до конца, поймете, почему), так что лицензию отдали мне, мол, играйся, если интересно.

Что же, система оказалась полной противоположностью Klocwork-у. Запуск из командной строки, все настройки через параметры или файл конфигурации, список файлов для анализа нужно готовить вручную. Правда, с Visual Studio существовала какая-никакая интеграция, которая, правда, всего лишь позволяла избежать создания списка файлов. Файлы конфигурации по-прежнему необходимо было писать «руками» в текстовом редакторе.

Вывод результатов – в текстовый файл. При запуске из Visual Studio – в консоль студии, откуда по клику можно было перейти на место предполагаемой проблемы (уже легче!).

Самая главная проблема PC-Lint – это количество сообщений. На относительно небольшом проекте он с установками по умолчанию выдал более 23 тысяч замечаний! Потратив пару дней на просмотр и «выключив» 34 наиболее частых и при этом вообще не критических класса проблем, добился снижения общего количества замечаний до пяти с половиной тысяч!

И конечно же, в такой куче легко может потеряться несколько реально важных замечаний, указывающих на реальное переполнение буфера или использование неинициализированной переменной.

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

«Выключать» можно только все сообщения определенного типа, возможности отметить одно конкретное сообщение как ложно-положительное нет никакой.

Конечно же, нет никакого трекинга от билда к билду. Можно ориентироваться только на общее количество сообщений.

Проблемы, найденные в заголовочных файлах, дублируются при каждом подключении этого файла. Если какой-то файл подключается десять раз, то все десять раз PC-Lint выведет сообщения обо всех найденных внутри проблемах. А если учесть, что проблемы он легко находит даже в файлах самой студии, особенно в реализации контейнеров STL…

При этом то, каким образом проблема возникает, также не показывается никак. Т.е. например он может сообщить, что в данном месте возможно использование неинициализированной переменной, но при каких именно условиях это происходит, приходится догадываться самому (в отличие от Klocwork, показывающего полный путь от начала функции к проблемной точке через все ветвления).

И в довершение – работает он только локально. При работе в команде нет никакой возможности легко и быстро сообщить о найденных проблемах остальным (только копи-паст текста, увы), как нет возможности и запуска всеми участниками (если, конечно, каждому участнику не будет куплена отдельная лицензия).

Есть, конечно, у PC-Lint и плюсы.

Количество сообщений можно при определенных условиях считать и плюсом. Несмотря на то, что большинство сообщений относятся к стилю и практически никогда не указывают на потенциальную проблему, стоит признать, что практически в каждом сообщении есть капля здравого смысла. И если (теоретически) исправить все, что PC-Lint предлагает исправить, код явно станет лучше, понятнее и читаемее. Но, как сказал кто-то в Интернете, «Я хочу программировать на С, а не на Lint!».

Пример «косметического» замечания:

Было:
calib_offset = (int32)(full_offset / full_gain * 100000);

PC-Lint здесь видит неоднозначность прочтения: a / b* c – это (a / b) * c или a / (b * c)?

Соответственно стало:
calib_offset = (int32)( (full_offset / full_gain) * 100000);

На мой взгляд, читаемость таки повысилась!

Если проект начинается с нуля и имеет не слишком большой размер, то при определенном желании можно и от PC-Lint добиться подхода «Ноль проблем». Если же на момент первого запуска проект близок к завершению, то «Ноль проблем» достигнуть будет практически нереально.

Из других плюсов я могу выделить следующее:

  • очень неплохое описание по каждому выдаваемому предупреждению (оформлены они все в отдельном большом пдф-документе, автоматически перейти к описанию из студии, естественно, невозможно);
  • очень большой набор правил, например, присутствуют правила, написанные по обеим книгам Майерса “Effective C++”;
  • возможность гибкой настройки – можно включать и выключать каждое правило по отдельности;
  • хорошо находит реальные проблемы (сложно только их найти среди обилия других сообщений);
  • цена – сильно дешевле остальных вариантов ($389 за неограниченную по времени лицензию).

И напоследок – пара реально найденных проблемных мест:

Было:

    switch (SocketID)
    {
      case SOCKETID_1:
      case SOCKETID_2:
        break;
      case SOCKETID_3:
        doSomething();
        break;
      case SOCKETID_4:
        doSomethingElse();
      case SOCKETID_5:
        doSomethingElseSimilar();
      case SOCKETID_6:
        doAnotherThing();
        break;
      default:
        return ERR_SERVICE_NOT_SUPPORTED;
    }

Стало:

    switch (SocketID)
    {
      case SOCKETID_1:
      case SOCKETID_2:
        break;
      case SOCKETID_3:
        doSomething();
        break;
      case SOCKETID_4:
        doSomethingElse();
        break;
      case SOCKETID_5:
        doSomethingElseSimilar();
        break;
      case SOCKETID_6:
        doAnotherThing();
        break;
      default:
        return ERR_SERVICE_NOT_SUPPORTED;
    }

Если вдруг кто-то не заметил, там было дважды пропущено слово break;

void main_task(void *p_arg)
{
  int status;

  // somewhere below…

  status = memory_init(&memory_config[0], memory_heap, sizeof(memory_heap));

  // and later “status” in never used…
}

Замечание к этому куску кода скорее стилистическое — «Значение переменной status нигде не используется», но, на мой взгляд, не столь бессмысленное – такой код создает ложную уверенность, что результат функции memory_init где-то учитывается, что, однако, не так.

И гораздо «честнее» переписать код вот так, не объявляя неиспользуемой переменной и явно отбрасывая результат.
void main_task(void *p_arg)
{
  // some code…

  memory_init(&memory_config[0], memory_heap, sizeof(memory_heap));

  // some other code
}

Конечно же, более правильным было бы проверить результат и выполнить необходимые действия в случае неудачи – но… это хорошо, когда проект находится на ранней стадии, и менять архитектуру приложения еще достаточно легко, а не когда проект уже завершен и в данном месте обработка исключительных ситуаций не была заложена архитектурно изначально (Проверить-то результат можно, а дальше что? Куда его потом девать, если он сигнализирует о неудаче, а вызывающая функция не имеет механизма обработки исключительных ситуаций?).

И именно для этого и нужен статический анализатор кода, запускаемый регулярно (идеально – после каждого билда!), вместе с требованием «Ноль предупреждений от анализатора».

Разумеется, никакой анализатор не заменит грамотного разработчика, code review и других техник и методов управления разработкой. Однако серьезно облегчить жизнь всей команде при грамотном использовании он может однозначно!
Tags:static code analysispc-lint
Hubs: Designing and refactoring
+23
9.1k 21
Comments 61
Popular right now
Профессия iOS-разработчик
November 30, 202075,000 ₽SkillFactory
Основы HTML и CSS
November 30, 2020FreeНетология
Frontend-разработчик с нуля
November 30, 202077,940 ₽Нетология
SMM-менеджер
November 30, 202059,998 ₽GeekBrains
Курс по аналитике данных
November 30, 202053,500 ₽SkillFactory
Top of the last 24 hours