How to become an author
Search
Write a publication
Pull to refresh

Comments 36

Ага, спасибо.

DHT/AAHD — это новый (contributed) код, я передам автору.
Последнее — не ошибка (стиль, понятно, херовый, но там унаследованный код, который «весь такой»)

Про сдвиги отрицательных чисел — подумаю. Это опять унаследованный код, но так как LibRaw начали нести с Intel на всякие другие архитектуры — может быть стоит вместо -1 явно написать 0xffffff
Total votes 18: ↑18 and ↓0+18
В некоторых случаях лучше написать явно. Компилятору пофиг, а читающему — приятнее.
Total votes 5: ↑4 and ↓1+3
Не знаю какой точно тип у maximum, но как же переносимость?
Total votes 2: ↑0 and ↓2-2
При чём здесь maximum…
Обсуждалось выражение (-1 << nbits):
Про сдвиги отрицательных чисел — подумаю. Это опять унаследованный код, но так как LibRaw начали нести с Intel на всякие другие архитектуры — может быть стоит вместо -1 явно написать 0xffffff

Я предложил заменить -1 на (~0u), чтобы избавиться от неопределённого поведения. Напомню:

2. The value of E1 << E2 is E1 left-shifted E2 bit positions; vacated bits are zero-filled. If E1 has an unsigned type, the value of the result is E1 * 2^E2, reduced modulo one more than the maximum value representable in the result type. Otherwise, if E1 has a signed type and non-negative value, and E1*2^E2 is representable in the result type, then that is the resulting value; otherwise, the behavior is undefined.

Кто-то, не разобравшись, начал минусовать комментарий. Люди ведь могут подумать, что я неправду написал. Прошу читать мои статьи и комментарии внимательно.
Total votes 3: ↑3 and ↓0+3
Я думаю, кто-то просто спросонья не понял, что это означает. А вообще инверсия беззнакового нуля — это действительно гарантированый способ получить единичку во все поляразряды, сколько бы их ни было.

Пользуясь случаем, добавлю, что этот пост — первый раз когда я увидел реальную пользу от вашего продукта. Приятно видеть, что вы помогаете людям (в том числе проектам, которыми я пользуюсь) :)
Total votes 3: ↑3 and ↓0+3
Прошу прощения, спросонья не понял, подумалось, что про другой кусок кода речь.

Вы правы в обоих случаях — сдвиг отрицательных чисел это UB, ну и (~0u) однозначно лучше.

А в комментарии моём имелось ввиду, что: «0x… не известно сколько F» это и читается плохо, и с переносимостью проблемы будут гарантированы.
This comment wasn’t rated yet0
А вот просто интересно услышать аргументы тех, кто ставит минусы для (~0u). Что не понравилось?
Total votes 2: ↑2 and ↓0+2
> DHT/AAHD — это новый (contributed) код, я передам автору.

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

После запуска статического анализатора, чаще всего ляпы будут найдены в новом коде или в крайне редко используемом коде (например, обработчиках ошибок). А остальное уже поправлено. Это я делаю намек на то, что настоящая польза от анализа будет получена при регулярных проверках, а не при «разовых наскоках».
Total votes 2: ↑2 and ↓0+2
Волшебная хабрасила — автор проекта ответил на замечания через 15 минут :)
Total votes 8: ↑8 and ↓0+8
А мне автор поста в моем блоге в комментариях написал, что вот есть пост на хабре.
Total votes 6: ↑6 and ↓0+6
Слава богу, здесь не будет вопроса, сообщил ли я авторам библиотеки о найденных недочётах. :)
Total votes 28: ↑28 and ↓0+28
Но есть еще много незаданных вопросов…
Total votes 3: ↑3 and ↓0+3
Не оставим, им шанса.
Евгений, почему пока нет версии под Linux?
Total votes 43: ↑41 and ↓2+39
И есть ли скидка для опенсорсных проектов!?
Total votes 2: ↑2 and ↓0+2
cppcheck говорит что [LibRaw-0.16.0/internal/dcraw_common.cpp:265]: (error) Uninitialized variable: c

    244 void CLASS canon_600_coeff()
    245 {
    246   static const short table[6][12] = {
    247     { -190,702,-1878,2390,   1861,-1349,905,-393, -432,944,2617,-2105  },
    248     { -1203,1715,-1136,1648, 1388,-876,267,245,  -1641,2153,3921,-3409 },
    249     { -615,1127,-1563,2075,  1437,-925,509,3,     -756,1268,2519,-2007 },
    250     { -190,702,-1886,2398,   2153,-1641,763,-251, -452,964,3040,-2528  },
    251     { -190,702,-1878,2390,   1861,-1349,905,-393, -432,944,2617,-2105  },
    252     { -807,1319,-1785,2297,  1388,-876,769,-257,  -230,742,2067,-1555  } };
    253   int t=0, i, c;
    254   float mc, yc;
    255 
    256   mc = pre_mul[1] / pre_mul[2];
    257   yc = pre_mul[3] / pre_mul[2];
    258   if (mc > 1 && mc <= 1.28 && yc < 0.8789) t=1;
    259   if (mc > 1.28 && mc <= 2) {
    260     if  (yc < 0.8789) t=3;
    261     else if (yc <= 2) t=4;
    262   }
    263   if (flash_used) t=5;
    264   for (raw_color = i=0; i < 3; i++)
    265     FORCC rgb_cam[i][c] = table[t][i*4 + c] / 1024.0;
    266 }


и судя повсему действительно не инициализирована… почему cppcat ничего не сказал об этом?
Total votes 2: ↑2 and ↓0+2
почему cppcat ничего не сказал об этом?
Потому, что CppCat лучше, чем Cppcheck.

Он раскрывает макрос FORCC и видит, что это:

#define FORC(cnt) for (c=0; c < cnt; c++)
#define FORCC FORC(colors)

При раскрытии кода получаем:

for (raw_color = i=0; i < 3; i++)
  for (c=0; c < colors; c++)
    rgb_cam[i][c] = table[t][i*4 + c] / 1024.0;


Спасибо, что помогли порекламироваться. :)
Total votes 20: ↑17 and ↓3+14
мне же не жалко, для хорошего человека то :)
Total votes 1: ↑1 and ↓0+1
>#define FORC(cnt) for (c=0; c < cnt; c++)

это же ужасно, за что разработчик так ненавидит тех кто позже будет сопровождать код… главное непонятно где тут выигрыш…
This comment wasn’t rated yet0
А пристрастие к такому стилю программирования обязательно рано или поздно приведёт к появлению в коде после препроцессинга какого-нибудь бреда вроде:
if(strcmp(strdup(a), strdup(b)) == 0)

Как правило сторонние разработчики, когда делают патчи, не всегда вдаются в тонкости устройства чужих дефайнов.

И такие ошибки cppcheck уже находил.
This comment wasn’t rated yet0
LibRaw — исторически — это перефигаченный полуавтоматическими средствами dcraw.c, чтобы из утилиты сделалась библиотека со стабильным интерфейсом. Потому что иначе «весь мир», ругаясь и матерясь, импортирует в себя эту самую dcraw.c (см. например UFRaw, RawTherapee из опенсорного. Точно ACDsee из коммерческого).

Как следствие, там где модификация исходного кода dcraw не была строго необходимой — она не делалась. Потому что безпроблемность/автоматичность импорта — важнее всех прочих соображений.

В сопровождение же кода кем-то, кто заглянул на полчасика со стороны — в данном случае я верю слабо.

Total votes 2: ↑2 and ↓0+2
Cppcheck из git HEAD уже не рапортует о такой ошибке:)

А вообще там есть ещё много подозрительных мест, так что провериться cppcheck-ом не мешает.

Ошибки вида (ret = f()) уже давно компиляторы отстреливают, это говорит лишь о том, что никто не читает предупреждения компилятора:)
This comment wasn’t rated yet0
Ошибки вида (ret = f()) уже давно компиляторы отстреливают, это говорит лишь о том, что никто не читает предупреждения компилятора:)
Может быть, я что-то делаю не так, но если просто открыть проект LibRaw, можно увидеть, что в настройках стоит /W0 (отключить все предупреждения).
Total votes 1: ↑1 and ↓0+1
и кстати да, почему так дорого?
Total votes 6: ↑3 and ↓30
Я дико извиняюсь, а что не дорого?
Total votes 4: ↑2 and ↓20
эт я не знаю, я просил когда то натравить на наш проект PVS-Studio, но что то не взлетело…
This comment wasn’t rated yet0
Да, мы общались про Midnight Commander в начале 2011 года. В то время проверка Linux проектов у нас была не очень (никак). С тех пор прошло много времени. Попробуйте Standalone версию PVS-Studio.
This comment wasn’t rated yet0
Можете проверить nginx? Было бы интересно почитать.
Total votes 8: ↑8 and ↓0+8
Кругом много проектов. Не обещаю, но может быть.
Total votes 7: ↑7 and ↓0+7
Чтобы было и тут тоже.
То что «признано» — внесено в бранч 0.16-stable у LibRaw и будет смержено в мастер (точнее, в приватной копии — уже, в паблик запушим как придет время).
То что «не ошибки, просто стиль такой» — оставлено как есть, см. комментарии выше.

This comment wasn’t rated yet0
Sign up to leave a comment.

Your account

Sections

Information

Services

Support
© 2006–2024, Habr