PVS-Studio corporate blog
January 2011 27

Intel IPP Samples for Windows — работа над ошибками

Проверка Intel IPP Samples for Windows
Это моя очередная заметка о том, как PVS-Studio делает программы более надёжными. То есть где, и какие ошибки он обнаруживает. На этот раз под молоток попали примеры, демонстрирующие работу с библиотекой IPP 7.0 (Intel Performance Primitives Library). Хотел, вначале, этот пост поместить в блог Intel, но потом решил, что это будет совсем уже...

В состав Intel Parallel Studio 2011 входит библиотека Performance Primitives Library. Эта библиотека включает в себя большое количество примитивов, позволяющих создавать эффективные видео и аудио кодеки, программы обработки сигналов, механизмы рендеринга изображений, архиваторы и многое-многое другое. Естественно, с такой библиотекой работать не просто. Поэтому компания Intel подготовила большое количество демонстрационных программ, построенных на основе этой библиотеки. Вы можете познакомиться с описанием примеров и скачать их по следующим ссылкам:

Все примеры разбиты на четыре группы:
  • IPP Samples for Windows
  • IPP UIC Demo for Windows
  • IPP DMIP Samples for Windows
  • IPP Cryptography Samples for Windows

В каждом наборе находится большое количество проектов, так что для начала я взял для проверки только первый набор IPP Samples for Windows. Проверку я осуществил, используя PVS-Studio версии 4.10.

Этим постом я хочу показать, что статический анализ полезен вне зависимости от профессионализма программистов и уровня разрабатываемого решения. Идея «надо брать профессионалов и писать сразу без ошибок» не работает. Даже высококвалифицированные разработчики не застрахованы от ошибок и опечаток в процессе кодирования. Ошибки в примерах для IPP очень хорошо это демонстрируют.

Подчеркну, что IPP Samples for Windows является высококачественным проектом. Однако, в силу своего размера в 1.6 миллионов строк кода, он неизбежно содержит в себе разнообразнейшие ошибки. Рассмотрим некоторые из них.

Неудачная замена индексов массива


Этот пример мог бы замечательно пополнить мою предыдущую статью "Последствия использования технологии Copy-Paste при программировании на Си++ и как с этим быть":
struct AVS_MB_INFO
{
  ...
  Ipp8u refIdx[AVS_DIRECTIONS][4];
  ...
};

void AVSCompressor::GetRefIndiciesBSlice(void){
  ...
  if (m_pMbInfo->predType[0] & predType)
  {
    m_refIdx[iRefNum] = m_pMbInfo->refIdx[dir][0];
    iRefNum += 1;
  }
  if (m_pMbInfo->predType[1] & predType)
  {
    m_refIdx[iRefNum] = m_pMbInfo->refIdx[dir][1];
    iRefNum += 1;
  }
  if (m_pMbInfo->predType[2] & predType)
  {
    m_refIdx[iRefNum] = m_pMbInfo->refIdx[dir][2];
    iRefNum += 1;
  }
  if (m_pMbInfo->predType[3] & predType)
  {
    m_refIdx[iRefNum] = m_pMbInfo->refIdx[dir][30];
    iRefNum += 1;
  }
  ...
}

Диагностическое сообщение PVS-Studio: V557 Array overrun is possible. The '30' index is pointing beyond array bound. avs_enc umc_avs_enc_compressor_enc_b.cpp 495

Программист несколько раз скопировал фрагмент кода и изменил значение индексов массивов. Но в самом конце его рука дрогнула. Он вписал число 3, но не удалил число 0. В результате получился индекс 30 и при исполнении кода произойдет доступ далеко за границы массива.

Одинаковые ветви кода


Раз мы начали с копирования кода, то вот ещё один пример на эту тему:
AACStatus aacencGetFrame(...)
{
  ...
  if (maxEn[0] > maxEn[1]) {
    ics[1].num_window_groups = ics[0].num_window_groups;
    for (g = 0; g < ics[0].num_window_groups; g++) {
      ics[1].len_window_group[g] = ics[0].len_window_group[g];
    }
  } else {
    ics[1].num_window_groups = ics[0].num_window_groups;
    for (g = 0; g < ics[0].num_window_groups; g++) {
      ics[1].len_window_group[g] = ics[0].len_window_group[g];
    }
  }
  ...
}  

Диагностическое сообщение PVS-Studio: V523 The 'then' statement is equivalent to the 'else' statement. aac_enc aac_enc_api_fp.c 1379

Но в этот раз, наоборот, забыли отредактировать скопированный код. Обе ветки условного оператора «if» выполняют одни и те же действия.

Путаница с приоритетом операции декремента "--" и разыменованием указателя "*"


static void
sbrencConflictResolution (..., Ipp32s *nLeftBord)
{
  ...
  *nLeftBord = nBordNext - 1;
  ...
  if (*lenBordNext > 1) {
    ...
    *nLeftBord--;
  }
  ...
}

Диагностическое сообщение PVS-Studio: V532 Consider inspecting the statement of '*pointer--' pattern. Probably meant: '(*pointer)--'. aac_enc sbr_enc_frame_gen.c 428

Указатель «nLeftBord» использует для возврата значения из функции «sbrencConflictResolution». Сначала по указанному адресу записывается значение «nBordNext — 1». При определенных условиях это значение должно уменьшаться на единицу. Для уменьшения значения программист использовал следующий код:
*nLeftBord--;

Ошибка в том, что вместо значения уменьшается сам указатель. Корректный код должен выглядеть так:
(*nLeftBord)--;

Ещё более запутанная ситуация с операцией инкремента "++" и разыменованием указателя "*"


Следующий код мне совершенно не понятен. Я не знаю, как его исправить, чтобы он приобрел смысл. Возможно, здесь чего-то не хватает.
static IppStatus mp2_HuffmanTableInitAlloc(Ipp32s *tbl, ...)
{
  ...
  for (i = 0; i < num_tbl; i++) {
    *tbl++;
  }
  ...
}

Диагностическое сообщение PVS-Studio: V532 Consider inspecting the statement of '*pointer++' pattern. Probably meant: '(*pointer)++'. mpeg2_dec umc_mpeg2_dec.cpp 59

Сейчас, приведенный в примере цикл, эквивалентен следующему коду:
tbl += num_tbl;

Анализатор PVS-Studio предположил, что, возможно, здесь забыты скобки и следовало написать "(*tbl)++;". Но и этот код не имеет смысла. Тогда цикл будет эквивалентен:
*tbl += num_tbl;

В общем, очень странный какой-то цикл. Ошибка есть, но исправить ее, видимо, может только автор кода.

Потеря признака, что произошла ошибка


В коде имеется функция «GetTrackByPidOrCreateNew», которая возвращает значение "-1", если возникает ошибка.
typedef signed int     Ipp32s;
typedef unsigned int   Ipp32u;

Ipp32s StreamParser::GetTrackByPidOrCreateNew(Ipp32s iPid, bool *pIsNew)
{
  ...
  else if (!pIsNew || m_uiTracks >= MAX_TRACK)
    return -1;
  ...
}

С функцией «GetTrackByPidOrCreateNew» всё нормально. Но имеется ошибка при её использовании:
Status StreamParser::GetNextData(MediaData *pData, Ipp32u *pTrack)
{
  ...
  *pTrack = GetTrackByPidOrCreateNew(m_pPacket->iPid, NULL);

  if (*pTrack >= 0 && TRACK_LPCM == m_pInfo[*pTrack]->m_Type)
    ippsSwapBytes_16u_I((Ipp16u *)pData->GetDataPointer(),
                        m_pPacket->uiSize / 2);
  ...
}

Диагностическое сообщение PVS-Studio: V547 Expression '* pTrack >= 0' is always true. Unsigned type value is always >= 0. demuxer umc_stream_parser.cpp 179

Значение, которое возвращает функция «GetTrackByPidOrCreateNew» сохраняется в беззнаковом формате (unsigned int). Это значит, что "-1" превратится в «4294967295», а условие "*pTrack >= 0" всегда истинно.

В итоге, если функция «GetTrackByPidOrCreateNew » вернет значение "-1", то произойдет Access Violation при выполнении кода «m_pInfo[*pTrack]->m_Type».

Copy-Paste и забытый +1


void H264SegmentDecoder::ResetDeblockingVariablesMBAFF()
{
  ...
  if (GetMBFieldDecodingFlag(m_gmbinfo->mbs[m_CurMBAddr 
                                            - mb_width * 2]))
    m_deblockingParams.nNeighbour[HORIZONTAL_DEBLOCKING] =
      m_CurMBAddr - mb_width * 2;
  else
    m_deblockingParams.nNeighbour[HORIZONTAL_DEBLOCKING] =
      m_CurMBAddr - mb_width * 2;
  ...
}

Диагностическое сообщение PVS-Studio: V523 The 'then' statement is equivalent to the 'else' statement. h264_dec umc_h264_segment_decoder_deblocking_mbaff.cpp 340

Если посмотреть код рядом, то становится понятно, что в скопированной строчке забыли прибавить единицу. Корректный код должен выглядеть так:
if (GetMBFieldDecodingFlag(m_gmbinfo->mbs[m_CurMBAddr 
                                          - mb_width * 2]))
  m_deblockingParams.nNeighbour[HORIZONTAL_DEBLOCKING] =
    m_CurMBAddr - mb_width * 2;
else
  m_deblockingParams.nNeighbour[HORIZONTAL_DEBLOCKING] =
    m_CurMBAddr - mb_width * 2 + 1;

Неподалеку в функции «H264CoreEncoder_ResetDeblockingVariablesMBAFF» есть ещё в точности такая же ошибка с забытым "+ 1".

Диагностическое сообщение PVS-Studio: V523 The 'then' statement is equivalent to the 'else' statement. h264_enc umc_h264_deblocking_mbaff_tmpl.cpp.h 366

Remove, который ничего не удаляет


void H264ThreadGroup::RemoveThread(H264Thread * thread)
{
    AutomaticUMCMutex guard(m_mGuard);
    std::remove(m_threads.begin(), m_threads.end(), thread);
}

Диагностическое сообщение PVS-Studio: V530 The return value of function 'remove' is required to be utilized. h264_dec umc_h264_thread.cpp 226

Интересное сочетание. С одной стороны, всё солидно. Используется mutex, чтобы корректно удалять элемент в многопоточном приложении. А с другой стороны, банально забыли, что функция std::remove не удаляет элементы из массива, а только меняет их порядок. На самом деле должно быть написано так:
m_threads .erase(
  std::remove(m_threads.begin(), m_threads.end(), thread),
  m_threads.end());

Сравнение полей структур с самими собой


Смотрю на ошибки и обратил внимание, что реализация стандарта сжатия видео H264 какая-то глючноватая. К этому проекту относится достаточно большое количество найденных ошибок. Вот, например, кто-то спешил при программировании и использовал сразу два неверных имени переменных.
bool H264_AU_Stream::IsPictureSame(H264SliceHeaderParse & p_newHeader)
{
  if ((p_newHeader.frame_num != m_lastSlice.frame_num) ||
      (p_newHeader.pic_parameter_set_id !=
       p_newHeader.pic_parameter_set_id) ||
      (p_newHeader.field_pic_flag != p_newHeader.field_pic_flag) ||
      (p_newHeader.bottom_field_flag != m_lastSlice.bottom_field_flag)
      ){
      return false;
  }
  ...
}

Диагностические сообщения PVS-Studio:

V501 There are identical sub-expressions 'p_newHeader.pic_parameter_set_id' to the left and to the right of the '!=' operator. h264_spl umc_h264_au_stream.cpp 478

V501 There are identical sub-expressions 'p_newHeader.field_pic_flag' to the left and to the right of the '!=' operator. h264_spl umc_h264_au_stream.cpp 479

Функция сравнения не работает, так как некоторые члены структуры сравниваются сами с собою. Вот две исправленные строчки:
(p_newHeader.pic_parameter_set_id != m_lastSlice.pic_parameter_set_id)
||
(p_newHeader.field_pic_flag != m_lastSlice.field_pic_flag) ||

Некорректное копирование данных


Бывают ошибки использования не тех объектов, не только при сравнении, но и при копировании состояний объектов:
Ipp32s ippVideoEncoderMPEG4::Init(mp4_Param *par)
{
  ...
  VOL.sprite_width = par->sprite_width;
  VOL.sprite_height = par->sprite_height;
  VOL.sprite_left_coordinate = par->sprite_left_coordinate;
  VOL.sprite_top_coordinate = par->sprite_left_coordinate;
  ...
}

Диагностическое сообщение PVS-Studio: V537 Consider reviewing the correctness of 'sprite_left_coordinate' item's usage. mpeg4_enc mp4_enc_misc.cpp 387

В «VOL.sprite_top_coordinate» помещается неверное значение. Корректное присваивание должно выглядеть так:
VOL.sprite_top_coordinate = par->sprite_top_coordinate;

Два цикла по одной переменной


JERRCODE CJPEGDecoder::DecodeScanBaselineNI(void)
{
  ...
  for(c = 0; c < m_scan_ncomps; c++)
  {
    block = m_block_buffer + (DCTSIZE2*m_nblock*(j+(i*m_numxMCU)));

    // skip any relevant components
    for(c = 0; c < m_ccomp[m_curr_comp_no].m_comp_no; c++)
    {
      block += (DCTSIZE2*m_ccomp[c].m_nblocks);
    }
  ...
}

Диагностическое сообщение PVS-Studio: V535 The variable 'c' is being used for this loop and for the outer loop. jpegcodec jpegdec.cpp 4652

Для двух циклов, вложенных друг в друга, используется одна переменная 'c'. Результат работы такой функции декодирования может быть весьма странным и неожиданным.

Двойное присваивание для большей надежности


H264EncoderFrameType*
H264ENC_MAKE_NAME(H264EncoderFrameList_findOldestToEncode)(...)
{
  ...
  MaxBrefPOC = 
    H264ENC_MAKE_NAME(H264EncoderFrame_PicOrderCnt)(pCurr, 0, 3);
  MaxBrefPOC = 
    H264ENC_MAKE_NAME(H264EncoderFrame_PicOrderCnt)(pCurr, 0, 3);
  ...
}

Диагностическое сообщение PVS-Studio: V519 The 'MaxBrefPOC' object is assigned values twice successively. Perhaps this is a mistake. h264_enc umc_h264_enc_cpb_tmpl.cpp.h 784

Когда я увидел этот код, то мне вспомнился старый программистский анекдот:

— А почему у тебя в коде подряд два одинаковых GOTO стоят?

— А вдруг первый не сработает!

Данная ошибка, пожалуй, не критична, но все-таки это ошибка.

Код, который настораживает


AACStatus sbrencResampler_v2_32f(Ipp32f* pSrc, Ipp32f* pDst)
{
  ...
  k = nCoef-1;
  k = nCoef;
  ...
}

Диагностическое сообщение PVS-Studio: V519 The 'k' object is assigned values twice successively. Perhaps this is a mistake. aac_enc sbr_enc_resampler_fp.c 90

Это двойное присваивание настораживает гораздо больше, чем в предыдущем примере. Такое ощущение, что программист был не уверен в себе. Или решил, в начале, попробовать «nCoef-1», а потом «nCoef». Ещё это называют «программированием методом эксперимента». И в любом случае, это именно то место в коде, увидев которое следует задержаться и предаться размышлениям.

Минимальное значение, которое не совсем минимально


void MeBase::MakeVlcTableDecision()
{
  ...
  Ipp32s BestMV= IPP_MIN(IPP_MIN(m_cur.MvRate[0],m_cur.MvRate[1]),
                         IPP_MIN(m_cur.MvRate[2],m_cur.MvRate[3]));
  Ipp32s BestAC= IPP_MIN(IPP_MIN(m_cur.AcRate[0],m_cur.AcRate[1]),
                         IPP_MIN(m_cur.AcRate[2],m_cur.AcRate[2]));
  ...
}

Диагностическое сообщение PVS-Studio: V501 There are identical sub-expressions to the left and to the right of the '<' operator: (m_cur.AcRate [2]) < (m_cur.AcRate [2]) me umc_me.cpp 898

Вновь опечатка в индексе массива. Последний индекс должен быть 3, а не 2. Корректный вариант кода:
Ipp32s BestAC= IPP_MIN(IPP_MIN(m_cur.AcRate[0],m_cur.AcRate[1]),
                       IPP_MIN(m_cur.AcRate[2],m_cur.AcRate[3]));

Подобные ошибки неприятны тем, что код «почти работает». Ошибка проявит себя только в том случае, если минимальный элемент хранится в «m_cur.AcRate[3]». Такие ошибки любят проявлять себя не при тестировании, а у пользователя на их наборах входных данных.

Максимальное значение, которое не совсем максимально


С максимальными значениями тоже не всегда ладится:
Ipp32s ippVideoEncoderMPEG4::Init(mp4_Param *par)
{
  ...
  i = IPP_MAX(mBVOPsearchHorBack, mBVOPsearchHorBack);
  ...
}

Диагностическое сообщение PVS-Studio: V501 There are identical sub-expressions '(mBVOPsearchHorBack)' to the left and to the right of the '>' operator. mpeg4_enc mp4_enc_misc.cpp 547

Два раза используется переменная mBVOPsearchHorBack. На самом деле планировалось использовать mBVOPsearchHorBack и mBVOPsearchVerBack:
i = IPP_MAX(mBVOPsearchHorBack, mBVOPsearchVerBack);

Попадаем пальцем в небо


typedef struct
{
  ...
  VM_ALIGN16_DECL(Ipp32f)      nb_short[2][3][__ALIGNED(MAX_PPT_SHORT)];
  ...
} mpaPsychoacousticBlock;

static void mp3encPsy_short_window(...)
{
  ...
  if (win_counter == 0) {
    nb_s = pBlock->nb_short[0][3];
  }
  ...
}

Диагностическое сообщение PVS-Studio: V557 Array overrun is possible. The '3' index is pointing beyond array bound. mp3_enc mp3enc_psychoacoustic_fp.c 726

Здесь видимо простая опечатка. Случайно использовали индекс '3' вместо '2'. Последствия, думаю, понятны.

Ошибка, приводящая к замедлению скорости работы


void lNormalizeVector_32f_P3IM(Ipp32f *vec[3], Ipp32s* mask, 
                               Ipp32s len) {
  Ipp32s  i;
  Ipp32f  norm;

  for(i=0; i<len; i++) {
    if(mask<0) continue;
    norm = 1.0f/sqrt(vec[0][i]*vec[0][i]+
           vec[1][i]*vec[1][i]+
           vec[2][i]*vec[2][i]);
           vec[0][i] *= norm; vec[1][i] *= norm; vec[2][i] *= norm;
  }
}

Диагностическое сообщение PVS-Studio: V503 This is a nonsensical comparison: pointer < 0. ipprsample ippr_sample.cpp 501

Это красивый пример кода, который из-за ошибки работает медленнее, чем мог бы. Алгоритм должен нормализовать только те элементы, которые отмечены в массиве масок. Но приведенный код делает нормализацию всех элементов. Ошибка находится в условии «if(mask<0)». Здесь забыли использовать индекс «i». Указатель «mask» будет практически всегда больше или равен нулю, а, значит, мы обработаем все элементы.

Корректная проверка:
if(mask[i]<0) continue;

Результат вычитания всегда равен 0


int ec_fb_GetSubbandNum(void *stat)
{
    _fbECState *state=(_fbECState *)stat;
    return (state->freq-state->freq);
}

Диагностическое сообщение PVS-Studio: V501 There are identical sub-expressions to the left and to the right of the '-' operator: state->freq — state->freq speech ec_fb.c 250

Здесь из-за опечатки функция всегда будет возвращать значение 0. Что-то не то мы здесь вычитаем. Что нужно вычитать, я не знаю.

Некорректная обработка нехватки буфера


typedef unsigned int    Ipp32u;

UMC::Status Init(..., Ipp32u memSize, ...)
{
  ...
  memSize -= UMC::align_value<Ipp32u>(m_nFrames*sizeof(Frame));
  if(memSize < 0)
      return UMC::UMC_ERR_NOT_ENOUGH_BUFFER;
  ...
}

Диагностическое сообщение PVS-Studio: V547 Expression 'memSize < 0' is always false. Unsigned type value is never < 0. vc1_enc umc_vc1_enc_planes.h 200

Неверно, обрабатывается ситуация, когда размер буфера для работы недостаточен. Вместо возврата кода ошибки программа продолжит работу и скорее всего, аварийно завершится. Дело в том, что переменная «memSize» имеет тип «unsigned int». Следовательно, условие «memSize < 0» всегда ложно и мы продолжаем работу с буфером недостаточного размера.

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

Некорректная проверка и как следствие выход за границу массива


Ipp32u m_iCurrMBIndex;
VC1EncoderMBInfo* VC1EncoderMBs::GetPevMBInfo(Ipp32s x, Ipp32s y)
{
  Ipp32s row = (y>0)? m_iPrevRowIndex:m_iCurrRowIndex;
  return ((m_iCurrMBIndex - x <0 || row <0)? 0 :
    &m_MBInfo[row][m_iCurrMBIndex - x]);
}

Диагностическое сообщение PVS-Studio: V547 Expression 'm_iCurrMBIndex — x < 0' is always false. Unsigned type value is never < 0. vc1_enc umc_vc1_enc_mb.cpp 188

Переменная «m_iCurrMBIndex» имеет тип «unsigned». Из-за этого выражение «m_iCurrMBIndex — x» также имеет тип «unsigned». Следовательно, условие «m_iCurrMBIndex — x < 0» всегда ложно. Рассмотрим последствия.

Пусть переменная «m_iCurrMBIndex» равна 5, а переменная «x» равна 10.

Выражение «m_iCurrMBIndex — x» равно 5u — 10i = 0xFFFFFFFBu.

Условие «m_iCurrMBIndex — x < 0» имеет значение false.

Выполняется выражение «m_MBInfo[row][0xFFFFFFFBu]» и происходит выход за границу массива.

Ошибка использования тернарного оператора '?:'.


Тернарный оператор достаточно опасен, так как легко допустить ошибку. Тем не менее, программисты любят написать покороче и использовать интересную конструкцию языка. Язык Си++ наказывает за это.
vm_file* vm_file_fopen(...)
{
  ...
  mds[3] = FILE_ATTRIBUTE_NORMAL |
           (islog == 0) ? 0 : FILE_FLAG_NO_BUFFERING;
  ...
}

Диагностическое сообщение PVS-Studio: V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '|' operator. vm vm_file_win.c 393

Код должен составлять комбинацию из флагов FILE_ATTRIBUTE_NORMAL и FILE_FLAG_NO_BUFFERING. Но на самом деле элементу «mds[3]» всегда присваивается значение 0.

Программист забыл, что приоритет оператора "|" выше, чем приоритет оператора "?:". Получается, что в коде написано следующее выражение (обратите внимание на скобки):

(FILE_ATTRIBUTE_NORMAL | (islog == 0)) ?

0: FILE_FLAG_NO_BUFFERING;

Условие «FILE_ATTRIBUTE_NORMAL | (islog == 0)» всегда истинно и мы присваиваем элементу «mds[3]» значение 0.

Корректное выражение должно выглядеть так (вновь обратите внимание на скобки):
FILE_ATTRIBUTE_NORMAL |
  ((islog == 0)) ? 0 : FILE_FLAG_NO_BUFFERING);

Странная работа с массивом


AACStatus alsdecGetFrame(...)
{
  ...
  for (i = 0; i < num; i++) {
    ...
    *tmpPtr = (Ipp32s)((tmp << 24) + ((tmp & 0xff00) << 8) +
                      ((tmp >> 8) & 0xff00) + (tmp >> 24));
    *tmpPtr = *srcPrt;
    ...
  }
  ...
}

Диагностическое сообщение PVS-Studio: V519 The '* tmpPtr' object is assigned values twice successively. Perhaps this is a mistake. aac_dec als_dec_api.c 928

Предлагаю читателю самому изучить код и сделать выводы. Я опишу этот код только одним словом — «оригинально».

Паранормальные присваивания


static
IPLStatus ownRemap8u_Pixel(...) {
  ...
  saveXMask    = xMap->maskROI;
  saveXMask    = NULL;
  saveYMask    = yMap->maskROI;
  saveYMask    = NULL;  
  ...
}

Диагностические сообщения PVS-Studio:

V519 The 'saveXMask' object is assigned values twice successively. Perhaps this is a mistake. ipl iplremap.c 36

V519 The 'saveYMask' object is assigned values twice successively. Perhaps this is a mistake. ipl iplremap.c 38

Причина появления такого странного кода мне непонятна. Причем подобный блок повторяется в разных функциях 8 раз!

Встречаются и другие подозрительные присваивания одной переменной:
Ipp32s ippVideoEncoderMPEG4::Init(mp4_Param *par)
{
  ...
  mNumOfFrames = par->NumOfFrames;
  mNumOfFrames = -1;
  ...
}

Диагностическое сообщение PVS-Studio: V519 The 'mNumOfFrames' object is assigned values twice successively. Perhaps this is a mistake. mpeg4_enc mp4_enc_misc.cpp 276

Заключение


В этой статье перечислена только часть ошибок, обнаруженных мною в IPP Samples for Windows. Некоторые ошибки я не привел, так как они являются братьями-близнецами тех примеров, которые я рассмотрел в статье, и читать про них будет не интересно. Я не стал приводить здесь несущественные ошибки. Примером может служить assert(), у которого из-за опечатки условие всегда истинно. Многие участков кода я пропустил, так как просто не знаю, ошибка это или просто некрасиво написано. Однако я думаю, что описанных дефектов достаточно, чтобы показать сложность написания больших проектов даже профессиональными разработчиками.

Ещё раз сформулирую мысль, озвученную в начале статьи. Даже хороший программист не застрахован от опечаток, забывчивости, желания сделать Copy-Paste и ошибок в логике. Я думаю, в будущем ссылка на эту статью станет хорошим ответом людям, которые уверены что произнесение фразы «надо правильно писать код» защитит их от любых ошибок.

Удачи всем в ваших C/C++/C++0x проектах. И желаю вам находить побольше ошибок, используя так любимую мной методологию статического анализа.
+18
2k 11
Comments 15