metrica
Мы используем куки, чтобы пользоваться сайтом было удобно.
Хорошо
to the top
close form

Заполните форму в два простых шага ниже:

Ваши контактные данные:

Шаг 1
Поздравляем! У вас есть промокод!

Тип желаемой лицензии:

Шаг 2
Team license
Enterprise license
** Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности
close form
Запросите информацию о ценах
Новая лицензия
Продление лицензии
--Выберите валюту--
USD
EUR
RUB
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

close form
Бесплатная лицензия PVS‑Studio для специалистов Microsoft MVP
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

close form
Для получения лицензии для вашего открытого
проекта заполните, пожалуйста, эту форму
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

close form
Мне интересно попробовать плагин на:
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

close form
check circle
Ваше сообщение отправлено.

Мы ответим вам на


Если вы так и не получили ответ, пожалуйста, проверьте папку
Spam/Junk и нажмите на письме кнопку "Не спам".
Так Вы не пропустите ответы от нашей команды.

Вебинар: Трудности при интеграции SAST, как с ними справляться - 04.04

>
>
>
Проверка проекта Trans-Proteomic Pipeli…

Проверка проекта Trans-Proteomic Pipeline (TPP)

21 Авг 2012

Если честно, я не знаю, для чего предназначен проект TPP. Как мне кажется, это набор инструментов для изучения белков и их взаимодействия в живых организмах. Впрочем, это не так важно. Главное, что исходные коды открыты. Значит, я могу проверить их с помощью статического анализатора PVS-Studio. А я люблю это делать.

Итак, был проверен проект Trans-Proteomic Pipeline (TPP) версии 4.5.2. Подробнее о проекте можно узнать по следующим ссылкам:

Мы пишем не обо всех проверенных проектах. Проект должен чем-то выделяться. В противном случае, описание найденных ошибок будет скучным. Например, проект должен быть известным, или содержать много ошибок определенного рода, или что-то ещё. Проект TPP обратил на себя внимание разнообразностью ошибок. Как правило, в проектах преобладают те или иные виды дефектов. В случае TPP мало повторяющихся ошибок. Они разнообразны и это интересно.

Следует отметить, что многие ошибки относятся не к самому проекту TPP, а к используемой в нём библиотеке для работы с XML. Однако я не думаю, что есть разница, будет неправильно обработан файл по вине программы или библиотеки XML. С точки зрения пользователя ошибка будет относиться к библиотеке TPP. Поэтому и я не буду уточнять, к какой части проекта относится тот или иной дефект. И хватит мне болтать. Давайте скорее преступим к интересным примерам.

Я переживаю, как сравниваются пептиды

К сожалению, я не знаю, что такое пептиды. Википедия подсказывает мне, что это семейство веществ, молекулы которых построены из остатков a-аминокислот, соединённых в цепь пептидными (амидными) связями -C(O)NH-. Вполне ожидаемо, что в TPP есть класс, названный Peptide, а в нём оператор сравнения. Реализован он следующим образом:

bool Peptide::operator==(Peptide& p) {
  ...
  for (i = 0, j = 0;
       i < this->stripped.length(), j < p.stripped.length();
       i++, j++) { 
  ...
}

PVS-Studio: V521 Such expressions using the ',' operator are dangerous. Make sure the expression is correct. tpplib peptide.cpp 191

Обратите внимание, что два сравнения записаны через оператор запятая ','. Оператор запятая возвращает значение выражения, расположенное справа. Это значит, что проверяется только одно условие: "j < p.stripped.length()". Корректное сравнение должно выглядеть следующим образом:

for (i = 0, j = 0;
     i < this->stripped.length() && j < p.stripped.length();
     i++, j++) {

Аналогичная ошибка допущена в методе Peptide::strippedEquals(). Я беспокоюсь за пептиды :).

Добавление лишней косой к файловым путям

Когда в программе работают с файловыми путями, часто нужно, чтобы путь заканчивался на косую \ или /. Для этого написан следующий код:

bool TandemResultsParser::writePepXML(....)
{
  ...
  char c = pathSummary.at(pathSummary.length() - 1);
  if (c != '\\' || c != '/')
  {
    if (pathSummary.find('\\') != string::npos)
      pathSummary.append("\\");
    else
      pathSummary.append("/");
  }
  ...
}

PVS-Studio: V547 Expression 'c != '\\' || c != '/'' is always true. Probably the '&&' operator should be used here. Tandem2XML tandemresultsparser.cxx 787

Если внимательно посмотреть на условие if (c != '\\' || c != '/'), то можно заметить опечатку. Условие всегда истинно. Переменная 'c' будет не равна '\\' или не равна '/'. В результате, в конце пути может появиться две косых. Возможно, эта ошибка не критична, но неприятна.

Корректное условие:

if (c != '\\' && c != '/')

Ошибка анализа пептидов

Рассмотрим код, который должен найти в строке подстроку " PI ":

class basic_string
{
  ...
  size_type find(const _Elem *_Ptr, size_type _Off = 0) const
  ...
}

void PipelineAnalysis::prepareFields(void) {
  ...
  if (peptideProphetOpts_.find(" PI ", 0)>=0) {
    fields_.push_back(Field("PpI_zscore"));
  }
  ...
}

PVS-Studio: V547 Expression 'peptideProphetOpts_.find(" PI ", 0) >= 0' is always true. Unsigned type value is always >= 0. pepXMLViewer pipelineanalysis.cxx 1590

Ошибочно используется функция std::string::find(). Если подстрока не находится, функция find() возвращает значение string::npos. Причем, это значение имеет беззнаковый тип.

В программе же предполагается, что если подстрока не найдена, функция find() должна вернуть отрицательное число. Это никогда не произойдёт. Условие "peptideProphetOpts_.find(" PI ", 0)>=0" всегда истинно, ведь беззнаковое значение всегда >= 0.

В результате, в независимости от того что содержит строка 'peptideProphetOpts', мы всегда пометим её как "PpI_zscore". И ещё, аналогичная ошибка находится в этой же функции чуть ниже (строка 1593). Я вновь волнуюсь за пептиды.

Корректный поиск подстроки должен выглядеть так:

if (peptideProphetOpts_.find(" PI ", 0) != string::npos)

Слишком случайный генератор случайных чисел

Фрагмент кода, генерирующий случайные числа может привести к гораздо большему количеству случайностей, чем хочется. Рассмотрим код:

int main(int argc, char **argv) {
  ...
  char salt[3];
  ...
  salt[0] = (argc>2)?(argv[1][0]):rndChar[rand() % 64];
  salt[1] = (argc>2)?(argv[1][1]):rndChar[rand() % 64];
  salt[3] = 0;
  ...
}

PVS-Studio: V557 Array overrun is possible. The '3' index is pointing beyond array bound. crypt crypt.cxx 567

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

Исправленный код:

salt[2] = 0;

Опечатка в индексе массива

void DIGEST_PROTEIN(char *szSeq, int iLenSeq)
{
  ...
  if (pOptions.bMarkNXST
      && szPeptide[i] == 'N'
      && szPeptide[i + 1] != 'P'
      && (szPeptide[i + 2] == 'S' ||
          szPeptide[i + 2] == 'T')
      && szPeptide[i + 1] != 'P')
  ...
}

PVS-Studio: V501 There are identical sub-expressions 'szPeptide[i + 1] != 'P'' to the left and to the right of the '&&' operator. Comet_fastadb comet-fastadb1.cxx 1906

Ячейка массива 'szPeptide[i + 1]' два раза сравнивается с 'P'. В результате тип пептида проверяется только приблизительно. Я думаю, в последней строке имеет место опечатка и код должен выглядеть так:

if (pOptions.bMarkNXST
    && szPeptide[i] == 'N'
    && szPeptide[i + 1] != 'P'
    && (szPeptide[i + 2] == 'S' ||
        szPeptide[i + 2] == 'T')
    && szPeptide[i + 3] != 'P')

Оформление кода не соответствует логике работы программы

Строчки в программе длинные, поэтому я заменил кое-что на многоточие. Не обращайте на это внимание. Ничего интересного я не удалил.

void MascotConverter::init(....) {
  ...
  if(line_len > 8 && .... && line[7] == '=')
    if(database_ == NULL)
      database_ = strCopy(line+8);
  else if(line_len > 5 && .... && line[4] == '=') {
    header_ = strCopy(line+5);
    ...
}

Посмотрите на 'else if'. Видите подвох? Оператор else относится ко второму, а не к первому оператору 'if'. Если правильно отформатировать код, он бы стал выглядеть так:

if(line_len > 8 && .... && line[7] == '=')
  if(database_ == NULL)
    database_ = strCopy(line+8);
  else if(line_len > 5 && .... && line[4] == '=') {
    header_ = strCopy(line+5);
    ...

Странная логика? Согласен. Скорее всего здесь логическая ошибка, а не ошибка оформления кода. Видимо, правильный код должен выглядеть так:

if(line_len > 8 && .... && line[7] == '=')
{
  if(database_ == NULL)
    database_ = strCopy(line+8);
}
else if(line_len > 5 && .... && line[4] == '=') {
  header_ = strCopy(line+5);
  ...

Вывод: не стоит жадничать и экономить место на фигурных скобках.

Неправильно инициализированные объекты

Чтобы не дублировать код, всегда есть соблазн позвать один конструктор из другого конструктора. К сожалению, при этом легко допустить ошибку. Вот как выглядит эта ошибка:

class ExperimentCycleRecord {
public:
  ExperimentCycleRecord()
    { ExperimentCycleRecord(0,0,0,True,False); }
  ...
}

PVS-Studio: V603 The object was created but it is not being used. If you wish to call constructor, 'this->ExperimentCycleRecord::ExperimentCycleRecord(....)' should be used. Mascot2XML mascotconverter.cxx 101

Высказывание "ExperimentCycleRecord(0,0,0,True,False);" создаёт временный объект и успешно его инициализирует. Но это не инициализирует поля текущего класса. Подробнее данный вид ошибки рассмотрен в статье: Не зная брода, не лезь в воду. Часть первая. В этой же статье предлагаются варианты, как исправить такие ошибки.

Аналогичные ошибки можно встретить ещё в некоторых местах:

  • V603 The object was created but it is not being used. If you wish to call constructor, 'this->ASAPRatioPeptideCGIDisplayParser::ASAPRatioPeptideCGIDisplayParser(....)' should be used. tpplib asapratiopeptidecgidisplayparser.cxx 36
  • V603 The object was created but it is not being used. If you wish to call constructor, 'this->ASAPRatioPeptideParser::ASAPRatioPeptideParser(....)' should be used. tpplib asapratiopeptideparser.cxx 57
  • V603 The object was created but it is not being used. If you wish to call constructor, 'this->CruxDiscrimFunction::CruxDiscrimFunction(....)' should be used. tpplib cruxdiscrimfunction.cxx 36
  • V603 The object was created but it is not being used. If you wish to call constructor, 'this->MascotDiscrimFunction::MascotDiscrimFunction(....)' should be used. tpplib mascotdiscrimfunction.cxx 47
  • V603 The object was created but it is not being used. If you wish to call constructor, 'this->MascotScoreParser::MascotScoreParser(....)' should be used. tpplib mascotscoreparser.cxx 37
  • V603 The object was created but it is not being used. If you wish to call constructor, 'this->TandemKscoreDF::TandemKscoreDF(....)' should be used. tpplib tandemkscoredf.cxx 37
  • V603 The object was created but it is not being used. If you wish to call constructor, 'this->TandemDiscrimFunction::TandemDiscrimFunction(....)' should be used. tpplib tandemdiscrimfunction.cxx 35
  • V603 The object was created but it is not being used. If you wish to call constructor, 'this->TandemNativeDF::TandemNativeDF(....)' should be used. tpplib tandemnativedf.cxx 37

Недописанный код

void TRANSLATE(int iFrame, char *szNewSeq,
      char *szSeq, int  *iLenSeq)
{
  ...
  *iLenSeq;
}

PVS-Studio: V607 Ownerless expression '* iLenSeq'. Comet_fastadb comet-fastadb1.cxx 2241

В конце функции 'TRANSLATE' есть непонятное высказывание "*iLenSeq;". Это высказывание ничего не делает. Возможно, это просто лишняя строчка. А возможно, здесь что-то не дописано. Но это вопрос, что именно...

Неинициализированные переменные

Вновь пришло время поволноваться о пептидах. Рассмотрим код:

void MixtureModel::assessPeptideProperties(char* filename, Boolean
  icat, Boolean glyc)
{
  ...
  double fval;
  ...
  // fval не инициализируется
  ...
  if(! icat && strstr(pep, "C") != NULL && fval >= min_fval) {
  ...
}

PVS-Studio: V614 Uninitialized variable 'fval' used. tpplib mixturemodel.cxx 834

Как поведёт себя проверка - неизвестно. Переменная 'fval' нигде не инициализируется.

А вот цикл, который будет выполняться неизвестное количество раз:

double mscore_c::dot_hr(unsigned long *_v)
{
  ...
  int iSeqSize;
  //perform a single pass through each array.
  //check every point in m_pfSeq,
  //but don't revisit positions in m_vmiType
  for (int a = 0; a < iSeqSize; a++) {
  ...
}

PVS-Studio: V614 Uninitialized variable 'iSeqSize' used. xtandem mscore_c.cpp 552

Переменная 'iSeqSize' не инициализирована.

Есть и другие неинициализированные переменные. Подробнее на них я останавливаться не буду и приведу просто их список:

  • V614 Uninitialized variable 'separator' used. pwiz sampledatum.hpp 95
  • V614 Uninitialized variable 'close' used. pwiz sampledatum.hpp 96
  • V614 Uninitialized variable 'threshold' used. pwiz spectrumlistfactory.cpp 497
  • V614 Uninitialized variable 'r' used. xtandem serialize.cpp 306
  • V614 Uninitialized variable 'fval' used. tpplib mixturemodel.cxx 840
  • V614 Uninitialized variable 'val' used. tpplib rtcalculator.cxx 735

Смотришь на всё это и удивляешься. И опасаешься. Интересные результаты могут выдавать научные исследования, основанные на неинициализированных переменных :).

Маленькая опечатка

Следующий фрагмент кода должен считать сумму элементов. Но из-за перепутанных местами двух символов, он этого не делает.

int main(int argc, char **argv)
{
  ...
  ii=0;
  for (i=0; pEnvironment.szPeptide[i]!=0; i++)
    ii =+ pEnvironment.szPeptide[i];
  ...
}

PVS-Studio: V588 The expression of the 'A =+ B' kind is utilized. Consider reviewing it, as it is possible that 'A += B' was meant. plot_msms plot-msms1.cxx 478

Ошибка элементарна. Однако она не перестаёт от этого быть ошибкой. Этот код хорошо демонстрирует, что многие дефекты в программах просты. Их намного больше, чем думают программисты. Подробнее об этот феномене я писал здесь: "Мифы о статическом анализе. Миф второй – профессиональные разработчики не допускают глупых ошибок".

Корректный код:

for (i=0; pEnvironment.szPeptide[i]!=0; i++)
  ii += pEnvironment.szPeptide[i];

Подозрительный итератор

Рассмотрим реализацию итератора.

CharIndexedVectorIterator& operator++()
{ // preincrement
  ++m_itr;
  return (*this);
}

CharIndexedVectorIterator& operator--()
{  // predecrement
  ++m_itr;
  return (*this);
}

PVS-Studio: V524 It is odd that the body of '--' function is fully equivalent to the body of '++' function (charindexedvector.hpp, line 68). pwiz charindexedvector.hpp 81

Оператор '++' написан правильно. А вот оператор '--' видимо писался методом Copy-Paste. В результате, он ведёт себя в точности также, как оператор '++'. Впрочем, другие операторы '--' также написаны подобным образом. Возможно это не ошибка, а хитрая задумка.

Цикл "на один раз"

Был найден цикл, который выполняется только один раз.

const char* ResidueMass::getStdModResidues(....) {
  ...
  for (rmap::const_iterator i = p.first; i != p.second; ++i) {
    const cResidue &r = (*i).second;
    if (r.m_masses[0].m_nterm) {
      n_term_aa_mod = true;
    } else if (r.m_masses[0].m_cterm) {
      c_term_aa_mod = true;
    }
    return r.m_residue.c_str();
  }
  ...
}

PVS-Studio: V612 An unconditional 'return' within a loop. tpplib residuemass.cxx 1442

В конце тела цикла имеется оператор 'return'. При этом видно, что в цикле нет оператора 'continue' или иных способов продолжить цикл. Это значит, что цикл выполняется только один раз. Мне сложно сказать, как должен выглядеть этот цикл. Возможно перед оператором 'return' необходимо вписать 'else'.

Странная инициализация

void ASAPCGIParser::writeProteinRatio(....)
{
  ...
  pvalue = (double)norm_->normalize(adj_inv_ratio);
    
  double tmp[2];
  tmp[0] = adj_inv_ratio[0];
  tmp[1] = adj_inv_ratio[1];
  adj_inv_ratio[0] = 1/ tmp[0];
  adj_inv_ratio[1] = tmp[1]/(tmp[0]*tmp[0]);

  pvalue = (double)norm_->normalize(adjratio);
  ...
}

PVS-Studio: V519 The 'pvalue' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 205, 214. tpplib asapcgiparser.cxx 214 (...)

Переменная 'pvalue' два раза подряд инициализируется разными значениями. Это подозрительно. Возможно, забыли инициализировать какую-то другую переменную.

И ещё про сравнение беззнаковых переменных с 0

Есть еще несколько дефектов, связанных со сравнением беззнаковых переменных с нулём. Например, встретилось ещё одно место, где не заладилась работа с косыми \, /.

int Dta2mzXML::extractScanNum(const string& dtaFileName)
{
  ...
  std::string::size_type pos = dtaFileName.rfind("/");

  if (pos < 0)  {
    pos = dtaFileName.rfind("\\");
  }
  ...
}

PVS-Studio: V547 Expression 'pos < 0' is always false. Unsigned type value is never < 0. dta2mzXML dta2mzxml.cpp 622

Переменная 'pos' всегда больше или равна 0. Мы уже рассматривали подобную ситуацию выше. Вот ещё несколько таких ошибок:

  • V547 Expression 'pos < 0' is always false. Unsigned type value is never < 0. dta2mzXML dta2mzxml.cpp 626
  • V547 Expression 'pos < 0' is always false. Unsigned type value is never < 0. dta2mzXML dta2mzxml.cpp 653
  • V547 Expression 'pos < 0' is always false. Unsigned type value is never < 0. dta2mzXML dta2mzxml.cpp 657

С функциями поиска разобрались. Осталось только пара ошибок по этой теме. Первая ошибка:

void SpectraSTReplicates::aggregateStats(....)
{
  ...
  unsigned int numAssignedPeaks =
    (*r)->entry->getPeakList()->getNumAssignedPeaks();
  if (numAssignedPeaks >= 0) {
    sumFracAssigned += (double)numAssignedPeaks/(double)numPeaks;
    numAnnotated++;
  }
  ...
}

PVS-Studio: V547 Expression 'numAssignedPeaks >= 0' is always true. Unsigned type value is always >= 0. tpplib spectrastreplicates.cpp 642

Думаю, комментарии и разъяснения здесь будут лишними. Вторая ошибка находится здесь:

V547 Expression 'pl->getNumAssignedPeaks() >= 0' is always true. Unsigned type value is always >= 0. tpplib spectrastreplicates.cpp 724

От условия ничего не зависит

Встретился подозрительный код, где вне зависимости от условия будет исполнен один и тот-же код. Возможно, это последствие Copy-Paste.

bool KernelDensityRTMixtureDistr::recalc_RTstats(....)
{
  ...
  if (catalog) {
    tmp = (*run_RT_calc_)[i]->recalc_RTstats(
      (*probs)[i], min_prob, (*ntts)[i], min_ntt, 2700);
  }
  else {
    tmp = (*run_RT_calc_)[i]->recalc_RTstats(
      (*probs)[i], min_prob, (*ntts)[i], min_ntt, 2700);
  }
  ...
}

PVS-Studio: V523 The 'then' statement is equivalent to the 'else' statement. tpplib kerneldensityrtmixturedistr.cxx 104

Формирование неправильного сообщения об ошибке

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

RAMPREAL *readPeaks(RAMPFILE *pFI,
      ramp_fileoffset_t lScanIndex)
{
  ...
  else
  {
    const char* pEndAttrValue;
    pEndAttrValue = strchr( pBeginData + 
        strlen( "contentType=\"") + 1 , '\"' );
    pEndAttrValue  = '\0';
    fprintf(stderr, "%s Unsupported content type\n" , pBeginData);
    return NULL;
  }
  ...
}

PVS-Studio: V527 It is odd that the '\0' value is assigned to 'char' type pointer. Probably meant: *pEndAttrValue = '\0'. tpplib ramp.cpp 1856

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

Корректный вариант кода:

*pEndAttrValue  = '\0';

Аналогичные ошибки можно заметить ещё в некоторых местах:

  • V527 It is odd that the '\0' value is assigned to 'char' type pointer. Probably meant: *pEndAttrValue = '\0'. tpplib ramp.cpp 1875
  • V527 It is odd that the '\0' value is assigned to 'char' type pointer. Probably meant: *pEndAttrValue = '\0'. spectrast spectrast_ramp.cpp 1844
  • V527 It is odd that the '\0' value is assigned to 'char' type pointer. Probably meant: *pEndAttrValue = '\0'. spectrast spectrast_ramp.cpp 1863

Неправильное вычисление длины массива

При записи XML файлов требуется сформировать временный буфер из 10 байт. Первый байт буфера должен быть равен '1', а остальные байты обнулены. Для этих целей удобно использовать функцию strncpy(). Описание функции strncpy:

char *strncpy (char *dst, const char *src, size_t len);

dst — Destination string.

src — Source string.

len — Number of characters to be copied.

The strncpy function copies the initial count characters of strSource to strDest and returns strDest. If count is less than or equal to the length of strSource, a null character is not appended automatically to the copied string. If count is greater than the length of strSource, the destination string is padded with null characters up to length count.

В библиотеке XML написан корректный на первый взгляд код:

void Out2XML::writeOutData() {
  ...
  // assume a string of less than
  // 9 characters will represent the charge state
  char *chg=(char*)malloc(10 * sizeof(char));
  //zero-fill the rest of the array
  strncpy(chg, "1", sizeof(chg));
  ...
}

PVS-Studio: V579 The strncpy function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. CombineOut out2xml.cxx 210

Ошибка заключается в том, что оператор sizeof() возвращает размер указателя, а вовсе не размер буфера. Несколько байт в конце останутся неинициализированными. Корректный вариант кода:

strncpy(chg, "1", 10); // zero-fill the rest of the array

Аналогичная ошибка:

V579 The strncpy function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. CombineOut out2xml.cxx 214

Неправильная проверка пустых строк

Можно быстро проверить, что строка пустая, сравнив первый её символ с нулём. Пример: str[0] == '\0'. Так часто делаю, но, к сожалению, иногда забывают разыменовывать указатель. Эти ошибки выглядят так:

void SAXSpectraHandler::pushPeaks(....)
{
  ...
  while(*pValue != '\0' && a < m_peaksCount) {
    while(*pValue != '\0' && isspace(*pValue))
      pValue++;
    if(pValue == '\0')
      break;
    m_vfM.push_back((float)atof(pValue));
    ...
}

PVS-Studio: V528 It is odd that pointer to 'char' type is compared with the '\0' value. Probably meant: *pValue == '\0'. xtandem saxhandler.cpp 323

Второй оператор while() служит для того, чтобы пропустить все пробелы. Затем нужно узнать, есть ли что-то после пробелов. Но указатель 'pValue' не разыменовывается и проверка никогда не срабатывает.

Корректный код:

if(*pValue == '\0')

Есть ещё парочка мест, где забыто разыменование указателя:

  • V528 It is odd that pointer to 'char' type is compared with the '\0' value. Probably meant: *pValue == '\0'. xtandem saxhandler.cpp 335
  • V528 It is odd that pointer to 'char' type is compared with the '\0' value. Probably meant: *pValue != '\0'. xtandem loadmspectrum.cpp 727
  • V528 It is odd that pointer to 'char' type is compared with the '\0' value. Probably meant: *pValue != '\0'. xtandem loadmspectrum.cpp 918

Неочищенные приватные данные

В библиотеке для работы с XML неаккуратно относятся к приватным данным. Не думаю, что это может как-то повредить TPP, однако раз ошибка найдена, её стоит описать.

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

Чтобы уничтожить приватные данные в буфере, в него нужно что-то записать. Многие используют для этих целей функцию memset(). Это плохая идея. Компилятор вправе удалить её вызов, если результат её работы никак не используется. Подробнее данная тема рассмотрена в документации: V597.

Пример опасного кода:

void CSHA1::Final()
{
  UINT_8 finalcount[8];
  ...
  memset(finalcount, 0, 8);
  Transform(m_state, m_buffer);
}

PVS-Studio: V597 The compiler could delete the 'memset' function call, which is used to flush 'finalcount' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. pwiz sha1.cpp 205

Компилятор вправе удалить вызов функции memset(), так как массив 'finalcount' после этого не используется.

Корректный код:

RtlSecureZeroMemory(finalcount, 8);

Аналогичные недоработки:

  • V597 The compiler could delete the 'memset' function call, which is used to flush 'finalcount' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. dta2mzXML sha1.cpp 252
  • V597 The compiler could delete the 'memset' function call, which is used to flush 'finalcount' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. indexmzXML indexmzxmlsha1.cpp 225

Прочее

Класс DiscriminantFunction содержит виртуальные функции, но при этом деструктор не объявлен виртуальным.

V599 The virtual destructor is not present, although the 'DiscriminantFunction' class contains virtual functions. tpplib discrimvalmixturedistr.cxx 201

Анализатор выдал много предупреждений V595. Это означает, что указатель вначале используется, а затем проверяется на нулевое значение. Это подозрительно, но далеко не всегда является ошибкой. Очень часто, указатель никак не может быть в этом месте нулевым. А проверка на 0 просто лишняя.

К сожалению, я ленив и не стал выяснять, где может произойти разыменование нулевого указателя, а где нет. Прощу прощения :). Думаю те, кого заинтересует эта статья, смогут это сделает самостоятельно, скачав PVS-Studio. Ещё отмечу, что в статью вошли далеко не все фрагменты кода, которые могут содержать ошибку. Очень сложно анализировать совершенно незнакомый проект.

Заключение

Когда я перечитал написанную статью, то у меня возникла ощущение, что после знакомства с ней у читателя может сложиться неправильное впечатление. Кажется, что статья выпячивает глупые ошибки и выставляет программистов не в лучшем свете. Нет, её смысл другой. Я хотел показать, что программирование это сложно. Очень сложно. Так сложно, что помимо алгоритмических ошибок можно наделать множество опечаток и прочих ляпов. И причина этих ляпов, вовсе не глупость или недостаточные знания. Слишком много при программировании приходится помнить и сосредотачиваться на множестве несвязанных задач. Всё это и приводит не только к сложным ошибкам, но и к простым. Со сложными может сразиться только программист. А вот многие простые, вполне может выявить инструмент статического анализа. При этом выявляя простые ошибки, он позволяет программисту большее внимание уделить алгоритмам и структуре программы. Не стоит игнорировать данный вид инструментария.

Получился длинный вывод. Поэтому ещё раз кратко и чётко:

  • Все программисты (даже профессионалы) делают ошибки;
  • Ошибки бывают простые и сложные;
  • Простых ошибок гораздо больше, чем думают программисты;
  • Большинство простых ошибок можно выявить статическими анализаторами кода;
  • Сокращая число простых ошибок, больше времени остается на исправление сложных ошибок и реализацию новых функций.
Популярные статьи по теме


Комментарии (0)

Следующие комментарии next comments
close comment form