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 проектах. И желаю вам находить побольше ошибок, используя так любимую мной методологию статического анализа.
    PVS-Studio
    589.85
    Static Code Analysis for C, C++, C# and Java
    Share post

    Comments 15

      +3
      Отлично! А если на какой-нибудь boost натравить вашу тулзу?
      Ох уж эти очепятки…
        +2
        IPP_MIN(IPP_MIN(m_cur.AcRate[0],m_cur.AcRate[1]),
                               IPP_MIN(m_cur.AcRate[2],m_cur.AcRate[3]));


        Странный приём. Вернее не странный даже, а опечаткоопаный. Что, собственно, и получили. Стараюсь вещи такого пода в макросы или функции выносить,
        — и писанины меньше, и шанс ошибки ниже
          +4
          Бл… А на ядро линукса — не? А то еще пачка эксплойтов будет.
            –9
            Слушай, как же ты заебал со своей тулзой, человек-с-красивым-ником. Не проходит и дня, как ты пишешь про нее пост. Купи себе блог корпоративный и пиши в него, тошно уже видеть это на главной.
            И да, выкини нахуй свои шрифты на заглавной картинке и не позорься.
              +1
              Без такого пиара классный продукт может загнуться. Цена адекватна большая :)
              Благодаря рекламе они смогут засветиться там где надо, тем кому надо.

              А в общем, в чем проблема? Не нравится — не читайте!

              По сабжу — ребята молодцы, продукт создали всем на зависть. Так держать!
                –7
                «не нравится» — не читайте: ализара, маркса, 95% корпоративных блогов, десяток блогов про стартапы, еще десяток новостников унылых, не читайте этих парней, ищущих халявы на ресурсе с вполне четкой коммерческой политикой.
                Кого читать-то тогда? Это я не я должен выбирать, что мне читать, я могу (или не могу) ставить оценки, которые слабо влияют на что-то, а сообщество.
                Топики в духе «смотрите, мы написали клёви продукт, но у нас нет денег на рекламу, потому что мы провинциальные нищеброды, поэтому давайте хавайте нашу рекламу с главной пару раз в неделю» должны здесь быть, по-вашему? Во что тогда превратится хабр еще через год-другой, если все софтовые конторки прохавают тут рекламную жилу?
                Я еще понимаю, если продукт был бы опенсорсным и топики на хабре приводили в его коммюнити людей, способных помочь развитию. Но тут исключительно нажива в интересах автора. Вам это нравится? Мне — нет.
                  +2
                  Выдыхай!
                    +3
                    Кхм) не хочешь читать — поставь минус. Если сообществу тема не интересна, то минус поставит много людей, таким образом топик не попадёт на главную страницу.

                    Пока какбе наоборот.
                +4
                Это очень хороший анализатор, работающий совершенно с новым режимом поиска ошибок.

                Например, вы много лет методично чистите свои проекты с помощью Lint + Visual Lint + свои тулзы, а потом запускаете PVS Studio.

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

                Лично мы не нарадуемся PVS Studio — это отличное дополнение к Lint и Intel Parallel Studio (все лицензионное).

                Надо поддерживать такие компании — отзыв от чистого сердца.
                  +3
                  А главное ведь еще — где вы найдете такую быструю и адекватную русскоязычную поддержку…

                  Спасибо за отзыв, мы Вас узнали и сильно любим :-).
                  +1
                  а я давно умер как программист — админ я
                  но всё равно читаю про PVS и интересно =)… хотя многое уже я не понимаю… поезд ушёл.
                  а у сказки будет счастливый конец? =) Все ошибки будут побеждены? =)
                    0
                    В реалистичных сказках побеждает зло. :)
                    0
                    кстает, а где галочку надо поставить чтоб проверялись файлы только в проекте, и не ругалось на тысячи ошибок в сторонних инклудах?
                    0

                    Only users with full accounts can post comments. Log in, please.