Андрей Карпов

07 Фев 2014

LibRaw, Coverity SCAN, PVS-Studio

• LibRaw
• Проверка с помощью Coverity SCAN
• Проверка с помощью PVS-Studio
  • Опечатки
  • Приоритет операций
  • Сдвиг отрицательных чисел
  • Странные фрагменты
• Заключение

Прочитал заметку о проверке маленького проекта LibRaw с помощью Coverity SCAN. Из статьи следует, что ничего интересного не нашлось. Решил попробовать, сможет ли найти что-то анализатор PVS-Studio.

LibRaw

LibRaw это библиотека для чтения RAW-файлов, получаемых с цифровых фотокамер (CRW/CR2, NEF, RAF, DNG и других). Сайт: http://www.libraw.org/

Проверка с помощью Coverity SCAN

А вот статья, которая подвигла меня на проверку проекта с помощью PVS-Studio: "Про статический анализ С++". Кратко процитирую основную часть статьи:

Coverity SCAN: 107 предупреждений, из них где-то треть - с High Impact.

Из High Impact:

Штук 10 в Microsoft STL

Еще какое-то количество такого примерно вида:

int variable;
if(layout==Layout1) variable=value1;
if(layout==Layout2) variable=value2;

И на это дается предупреждение, дескать неаккуратненько, не инициализированная переменная. Я с ним согласен по общим ощущениям, так не надо делать. Но в реальной жизни бывает два вида layout - и это явно прописано в вызывающем коде. Т.е. у машинки достаточно данных, чтобы сообразить, что это не 'High impact', а просто неаккуратненько.

Некоторое количество предупреждений, дескать unsigned short при расширении до 32-64 бит может больно укусить. С этим я не хочу спорить - формально машинка права, а по факту в этих unsigned short живут размеры картинки и до 32767 они в ближайшие годы не дорастут.

Т.е. опять, фиксить не надо - в случае данной предметной области.

Все остальные найденные 'High Impact' проблемы - это просто ложные срабатывания. Т.е. код, согласен, не идеальный (видели бы вы этот код из dcraw !), но все найденное - не является ошибкой.

Проверка с помощью PVS-Studio

Теперь посмотрим, удастся ли что-то найти после Coverity анализатору PVS-Studio. Конечно, никаких ожиданий об обнаружении супер-ошибок нет, но всё равно интересно попробовать.

Анализатор PVS-Studio выдал 46 предупреждений общего назначения (первый и второй уровень важности).

Предлагаю посмотреть на фрагменты кода, которые показалось мне интересными.

Опечатки

void DHT::hide_hots() {
  ....
  for (int k = -2; k < 3; k += 2)
    for (int m = -2; m < 3; m += 2)
      if (m == 0 && m == 0)
        continue;
      else
        avg += nraw[nr_offset(y + k, x + m)][kc];

  ....
}

Предупреждение PVS-Studio: V501 There are identical sub-expressions to the left and to the right of the '&&' operator: m == 0 && m == 0 dht_demosaic.cpp 260

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

if (k == 0 && m == 0)

Идентичный фрагмент также имеется в файле aahd_demosaic.cpp (строка 199).

Приоритет операций

int main(int argc, char *argv[])
{
  int ret;
  ....
  if( (ret = RawProcessor.open_buffer(iobuffer,st.st_size)
             != LIBRAW_SUCCESS))
  {
    fprintf(stderr,"Cannot open_buffer %s: %s\n",
      argv[arg],libraw_strerror(ret));
    free(iobuffer);
    continue;
  }
  ....
}

Предупреждение PVS-Studio: V593 Consider reviewing the expression of the 'A = B != C' kind. The expression is calculated as following: 'A = (B != C)'. dcraw_emu.cpp 468

Ошибка, связанная с приоритетами операций. В начале выполняется сравнение "RawProcessor.open_buffer(iobuffer,st.st_size) != LIBRAW_SUCCESS". Затем результат этого сравнения записывается в переменную 'ret'. Если возникнет ошибка, то в файл будет распечатан неправильный код ошибки. Не критичный недочёт, но всё равно стоит про него рассказать.

Сдвиг отрицательных чисел

unsigned CLASS pana_bits (int nbits)
{
  .... 
  return (buf[byte] | buf[byte+1] << 8) >>
         (vbits & 7) & ~(-1 << nbits);
  ....
}

Предупреждение PVS-Studio: V610 Undefined behavior. Check the shift operator '<<. The left operand '-1' is negative. dcraw_common.cpp 1827

Сдвиг отрицательных значений приводит к undefined behavior. Такими трюками часто пользуются, и программа делает вид что работает. Но на самом деле, полагаться на такой код нельзя. Подробнее про сдвиги отрицательных чисел можно прочитать здесь: Не зная брода, не лезь в воду. Часть третья.

Аналогичные сдвиги можно найти здесь:

• dcraw_common.cpp 1851
• dcraw_common.cpp 2085
• dcraw_common.cpp 2814
• dcraw_common.cpp 6644

Странные фрагменты

void DHT::illustrate_dline(int i) {
  ....
    int l = ndir[nr_offset(y, x)] & 8;
    l >>= 3;
    l = 1;
  ....
}

Предупреждение PVS-Studio: V519 The 'l' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 671, 672. dht_demosaic.cpp 672

Возможно это не ошибка и "l = 1" написано специально. Однако, код выглядит подозрительно.

Вот ещё одно подозрительное место:

void CLASS identify()
{
  ....
  if (!load_raw && (maximum = 0xfff))
  ....
}

Предупреждение PVS-Studio: V560 A part of conditional expression is always true: ((imgdata.color.maximum) = 0xfff). dcraw_common.cpp 8496

Заключение

Оба анализатора нашли очень мало. Это естественно для маленьких проектов. Однако, всё равно было интересно провести эксперимент по проверке LibRaw.

0

0

0

0

ПОДЕЛИТЬСЯ

Присылаем лучшие статьи раз в месяц