PVS-Studio хотел, но не смог найти баги в robots.txt

На днях Google опубликовал исходники парсера robots.txt. Почему бы не прогнать уже проверенный всеми вдоль и поперек проект через PVS-Studio и, возможно, найти ошибку. Сказано — сделано. Жаль, что ничего значимого найти не удалось. Ну что ж, тогда пусть это будет просто повод похвалить разработчиков Google.

robots.txt – индексный файл, который содержит правила для поисковых роботов. Он действует для протоколов https, http и FTP. Google сделала доступным для всех свой парсер файла robots.txt. Подробнее об этой новости можно почитать здесь: Google открывает исходный код парсера robots.txt ^[1]

Думаю, большинству читающих наши статьи известно, что делает PVS-Studio. Но на случай, если вы впервые в нашем блоге, дадим краткую справку. PVS-Studio – статический анализатор кода, который позволяет находить разнообразные ошибки, уязвимости и недочеты в проектах, написанных на С, С++, С# и Java. Другими словами, PVS-Studio является SAST ^[2] решением и может работать как на пользовательских машинах или сборочных серверах, так и в облаке ^[3]. А ещё команда PVS-Studio очень любит писать статьи ^[4] о проверке различных проектов. Так что перейдем к делу и попробуем найти ошибки в исходном коде парсера от Google.

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

В общем эта статья получилась в духе другой нашей публикации "Самая короткая статья о проверке nginx ^[5]".

Нашлась возможность небольшой оптимизации:

V805 ^[6] Decreased performance. It is inefficient to identify an empty string by using 'strlen(str) > 0' construct. A more efficient way is to check: str[0] != ''. robots.cc 354

bool RobotsTxtParser::GetKeyAndValueFrom(char **key, ....)
{
  ....  
  *key = line;
  ....
  if (strlen(*key) > 0) {
    ....
    return true;
  }
  return false;
}

Вызов функции strlen для того, чтобы узнать, является ли строка непустой — это неэффективный способ. Такую проверку можно произвести гораздо проще: if (*key[0] != ''), и не нужно будет проходить по всем элементам строки, если она непустая.

V808 ^[7] 'path' object of 'basic_string' type was created but was not utilized. robots.cc 123

std::string GetPathParamsQuery(....)
{
  std::string path;
  ....
}

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

В следующем случае анализатор, по сути, дает рекомендацию добавить дефолтный return после того, как весь main будет отработан. Возможно, стоило бы добавить return в самом конце, чтобы можно было понять, что все действительно отработало. Однако если данное поведение и было задумано, и изменять ничего не нужно, а сообщение анализатора видеть бы не хотелось, то в случае PVS-Studio можно подавить это предупреждение и никогда его больше не видеть :).

V591 ^[8] The 'main' function does not return a value, which is equivalent to 'return 0'. It is possible that this is an unintended behavior. robots_main.cc 99

int main(int argc, char** argv)
{
  ....
  if (filename == "-h" || filename == "-help" || filename == "--help")
  {
    ShowHelp(argc, argv);
    return 0;
  }
  if (argc != 4)
  {
    ....
    return 1;
  }
  if (....)
  {
    ....
    return 1;
  }
  ....
  if (....)
  {
    std::cout << "...." << std::endl;
  }
}

Также обнаружилось, что приведенные ниже две функции, имеющие разные имена, имеют одну и ту же реализацию. Возможно, это результат того, что раньше эти функции имели разную логику, но пришли к одной. А может быть и так, что где-то закралась опечатка, поэтому такие предупреждения стоит внимательно проверять.

V524 ^[9] It is odd that the body of 'MatchDisallow' function is fully equivalent to the body of 'MatchAllow' function. robots.cc 645

int MatchAllow(absl::string_view path, absl::string_view pattern)
{
  return Matches(path, pattern) ? pattern.length() : -1;
}

int MatchDisallow(absl::string_view path, absl::string_view pattern)
{
  return Matches(path, pattern) ? pattern.length() : -1;
} 

Это единственное место, которое вызывает у меня какое-то подозрение. Его стоит проверить авторам проекта.

Таким образом, проверка парсера robots.txt от Google показала, что столь активно используемый и, скорее всего, многократно проверенный на ошибки проект, имеет высокое качество кода. А найденные недочеты совсем не могут испортить впечатление от того, какие крутые кодеры из Google занимались этим проектом :).

Предлагаем и вам скачать и попробовать PVS-Studio ^[10] на интересующем вас проекте.

^[11]

Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Victoria Khanieva. PVS-Studio wanted but couldn't find bugs in robots.txt ^[12]

Автор: vkhanieva

Источник ^[13]