Статический анализ наиболее полезен при регулярных проверках. Особенно для таких активно развивающихся проектов как Blender. Пришло время проверить его вновь, и узнать, какие подозрительные места удастся найти на этот раз.Введение
Blender — профессиональный пакет для создания трёхмерной компьютерной графики, включающий в себя средства моделирования, анимации, рендеринга, постобработки и монтажа видео со звуком, также использующийся для создания интерактивных игр.
Проект уже проверялся ранее. Результаты проверки версии 2.62 изложены в статье "Проверка проекта Blender с помощью PVS-Studio".
Со времени прошлой проверки размер исходного кода вместе с дополнительными библиотеками увеличился до 77 мегабайт. А его объём вырос до 2206 KLOC. На момент предыдущей поверки размер проекта составлял 68 мегабайт (2105 KLOC).
Рассчитать объём кода мне помогла утилита SourceMonitor. Эта утилита умеет анализировать код на языках C++, C, C#, VB.NET, Java, Delphi и рассчитывает различные метрики. Например, она может определить цикломатическую сложность ваших проектов, а также сформировать подробную статистику для каждого из файлов проекта. И показать результаты в виде таблицы или диаграмм.
Итак, эта статья расскажет про ошибки и подозрительные места, найденные в версии Blender 2.77a. Для проверки использовался анализатор PVS-Studio версии 6.05.
Опечатки
При активном использовании механизмов копирования и автоматического дополнения кода очень часто могут возникнуть ошибки в наименованиях различных переменных и констант. Такие ошибки могут привести к неверным результатам вычислений или непредвиденному поведению программы. В проекте Blender анализатор нашёл сразу несколько примеров. Рассмотрим их подробно.
Опечатка в условии
CurvePoint::CurvePoint(CurvePoint *iA, CurvePoint *iB, float t3)
{
....
if ((iA->getPoint2D() - //<=
iA->getPoint2D()).norm() < 1.0e-6) { //<=
....
}
....
} V501 There are identical sub-expressions to the left and to the right of the '-' operator: iA->getPoint2D() — iA->getPoint2D() curve.cpp 136
Внутри функции CurvePoint происходит работа с двумя близкими по имени объектами iA и iB. Разные методы этих объектов постоянно пересекаются в различных операциях в довольно длинном дереве условий. В одном из условных блоков допущена опечатка. В результате происходит операция вычитания между свойством одного и того же объекта. Не зная особенностей кода, точно сказать, в каком из двух операндов допущена ошибка, невозможно. Предлагаю два возможных варианта её исправления:
if ((iA->getPoint2D()-iB->getPoint2D()).norm()<1.0e-6)....или
if ((iB->getPoint2D()-iA->getPoint2D()).norm()<1.0e-6)....Следующая ошибка тоже спряталась внутри условного оператора.
template<typename MatrixType, int QRPreconditioner>
void JacobiSVD<MatrixType, QRPreconditioner>::allocate(....)
{
....
if(m_cols>m_rows)m_qr_precond_morecols.allocate(*this);
if(m_rows>m_cols)m_qr_precond_morerows.allocate(*this);
if(m_cols!=m_cols)m_scaledMatrix.resize(rows,cols); //<=
}V501 There are identical sub-expressions to the left and to the right of the '!=' operator: m_cols != m_cols jacobisvd.h 819
В приведённом фрагменте кода происходит уравнение количества строк и столбцов внутри некой матрицы. Если количество соответственно не равно, происходит выделение памяти для новых элементов или их создание. А после, если были добавлены новые ячейки, происходит операция изменения размера матрицы. Но в результате ошибки в выражении условного оператора операция не произойдёт никогда, так как условие m_cols!=m_cols всегда ложно. В данном случае не имеет значения, какую из двух частей выражения требуется изменить, поэтому предлагаю такой вариант:
if(m_cols!=m_rows) m_scaledMatrix.resize(rows,cols)Ещё несколько проблемных мест, обнаруженных диагностикой V501:
- V501 There are identical sub-expressions to the left and to the right of the '==' operator: left.rows() == left.rows() numeric.cc 112
- V501 There are identical sub-expressions to the left and to the right of the '>' operator: (from[0][3]) > (from[0][3]) stereoimbuf.c 120
- V501 There are identical sub-expressions to the left and to the right of the '>' operator: (from[0][3]) > (from[0][3]) stereoimbuf.c 157
- V501 There are identical sub-expressions to the left and to the right of the '==' operator: out->y == out->y filter.c 209
Работа с нулевым указателем
Здесь опечатка в наименовании привела к более серьёзной ошибке.
int QuantitativeInvisibilityF1D::operator()(....)
{
ViewEdge *ve = dynamic_cast<ViewEdge*>(&inter);
if (ve) {
result = ve->qi();
return 0;
}
FEdge *fe = dynamic_cast<FEdge*>(&inter);
if (fe) {
result = ve->qi(); //<=
return 0;
}
....
}V522 Dereferencing of the null pointer 've' might take place. functions1d.cpp 107
Приведённая функция достаточно короткая, но опечатки могут подстерегать нас даже в простых функциях. Из кода видно, что последовательно создаются и проверяются два объекта. Но после проверки второго объекта возникла ошибка, и если fe успешно создан, то вместо него в результат записывается результат работы функции из первого объекта, который согласно ранним условиям не был создан. Это скорее всего приведёт к аварийному завершению программы, если исключение не перехватит обработчик более высокого уровня.
По всей видимости второй фрагмент кода писался с помощью Copy-Paste. И случайно в одном месте забыли поменять имя переменной ve. Правильный же код, скорее всего, должен был быть таким:
FEdge *fe = dynamic_cast<FEdge*>(&inter);
if (fe) {
result = fe->qi();
return 0;
}Использование нулевого указателя
static ImBuf *accessor_get_ibuf(....)
{
ImBuf *ibuf, *orig_ibuf, *final_ibuf;
....
/* First try to get fully processed image from the cache. */
ibuf = accesscache_get(accessor,
clip_index,
frame,
input_mode,
downscale,
transform_key);
if (ibuf != NULL) {
return ibuf;
}
/* And now we do postprocessing of the original frame. */
orig_ibuf = accessor_get_preprocessed_ibuf(accessor,
clip_index,
frame);
if (orig_ibuf == NULL) {
return NULL;
}
....
if (downscale > 0) {
if (final_ibuf == orig_ibuf) {
final_ibuf = IMB_dupImBuf(orig_ibuf);
}
IMB_scaleImBuf(final_ibuf,
ibuf->x / (1 << downscale), //<=
ibuf->y / (1 << downscale)); //<=
}
....
if (input_mode == LIBMV_IMAGE_MODE_RGBA) {
BLI_assert(ibuf->channels == 3 || //<=
ibuf->channels == 4); //<=
}
....
return final_ibuf;
}Предупреждения:
- V522 Dereferencing of the null pointer 'ibuf' might take place. tracking_util.c 765
- V522 Dereferencing of the null pointer 'ibuf' might take place. tracking_util.c 766
- V522 Dereferencing of the null pointer 'ibuf' might take place. tracking_util.c 783
В приведённом фрагменте видно, что проверка переменной ibuf в случае, если объект создан, прерывает функцию гораздо раньше, чем эта переменная используется. Возможно на этом можно было остановится и подтвердить факт разыменования указателя. Однако при более внимательном изучении кода и комментариев к нему, выясняется истинная причина ошибки. И на самом деле здесь опять допущена опечатка. В местах, указанных анализатором, на самом деле должна была использоваться переменная orig_ibuf вместо ibuf.
Неверный тип переменной
typedef enum eOutlinerIdOpTypes {
OUTLINER_IDOP_INVALID = 0,
OUTLINER_IDOP_UNLINK,
OUTLINER_IDOP_LOCAL,
....
} eOutlinerIdOpTypes;
typedef enum eOutlinerLibOpTypes {
OL_LIB_INVALID = 0,
OL_LIB_RENAME,
OL_LIB_DELETE,
} eOutlinerLibOpTypes;
static int outliner_lib_operation_exec(....)
{
....
eOutlinerIdOpTypes event; //<=
....
event = RNA_enum_get(op->ptr, "type");
switch (event) {
case OL_LIB_RENAME: //<=
{
....
}
case OL_LIB_DELETE: //<=
{
....
}
default:
/* invalid - unhandled */
break;
}
....
}Предупреждения:
- V556 The values of different enum types are compared: switch(ENUM_TYPE_A) { case ENUM_TYPE_B:… }. outliner_tools.c 1286
- V556 The values of different enum types are compared: switch(ENUM_TYPE_A) { case ENUM_TYPE_B:… }. outliner_tools.c 1295
В примере показаны два типа, являющиеся перечислениями. И то, что при написании кода в типе была допущена опечатка, вполне ожидаемо для таких, почти неотличимых по имени, типов.
Фактически код работает корректно. Но при этом он вводит в заблуждение несоответствием типов. Переменная получает значение одного перечисления и сравнивается с константами другого. Для исправления ошибки в данном случае достаточно изменить тип переменой event на eOutlinerLibOpTypes.
Ошибка приоритетов операций
static void blf_font_draw_buffer_ex(....)
{
....
cbuf[3] = (unsigned char)((alphatest = ((int)cbuf[3] +
(int)(a * 255)) < 255) ? alphatest : 255);
....
}V593 Consider reviewing the expression of the 'A = B < C' kind. The expression is calculated as following: 'A = (B < C)'. blf_font.c 414
Несоблюдение приоритета операций — одна из часто встречающихся ошибок при работе со сложными выражениями. В данном случае это всего лишь опечатка, но она привела к нарушению логики работы тернарного оператора. Из-за неверно поставленной скобки возникла ошибка с приоритетом операций. Вдобавок портится значение переменной alphatest. Вместо значения, которое вычисляет тернарный оператор, переменной alphatest присваивается значение bool-типа, полученное в результате выполнения операции сравнения (<). А уже после этого тернарный оператор работает со значением переменой alphatest, и результат его работы не сохраняется. Для исправления ошибки необходимо изменить выражение следующим образом:
cbuf[3] = (unsigned char)(alphatest = (((int)cbuf[3] +
(int)(a * 255)) < 255) ? alphatest : 255);Ошибка в константе
bool BKE_ffmpeg_alpha_channel_is_supported(RenderData *rd)
{
int codec = rd->ffcodecdata.codec;
if (codec == AV_CODEC_ID_QTRLE)
return true;
if (codec == AV_CODEC_ID_PNG)
return true;
if (codec == AV_CODEC_ID_PNG)
return true;
....
} V649 There are two 'if' statements with identical conditional expressions. The first 'if' statement contains function return. This means that the second 'if' statement is senseless. Check lines: 1672, 1675. writeffmpeg.c 1675
В функции проводится последовательная проверка значения переменой на соответствие флагу с помощью однострочных условий. В результате опечатки один из флагов проверяется дважды. Наверняка, вместо повторной проверки должна быть проверена другая константа. Но вариантов этих констант очень много, поэтому я не могу предположить, как именно надо исправить этот код.
Использование одной переменой во внешнем и вложенном циклах
bool BM_face_exists_overlap_subset(...., const int len)
{
int i;
....
for (i = 0; i < len; i++) {
BM_ITER_ELEM (f, &viter, varr[i], BM_FACES_OF_VERT) {
if ((f->len <= len) && (....)) {
BMLoop *l_iter, *l_first;
if (is_init == false) {
is_init = true;
for (i = 0; i < len; i++) { //<=
BM_ELEM_API_FLAG_ENABLE(varr[i], _FLAG_OVERLAP);
}
}
....
}
}
}
} V535 The variable 'i' is being used for this loop and for the outer loop. Check lines: 2204, 2212. bmesh_queries.c 2212
Использование одной и той же переменой во внешнем и вложенном цикле может привести к некорректному выполнению внешнего цикла. В данном случае это скорее всего не является ошибкой, так как цикл видимо ищет нужный элемент и завершается, и второй цикл срабатывает только в этом случае. Но всё равно использование единой переменой является опасным местом, и может привести к реальным ошибкам, если понадобится оптимизировать этот фрагмент кода.
Избыточный код
Лишние фрагменты кода можно встретить в любой программе. Иногда это старый код, оставшийся после рефакторинга. А бывает, что лишние фрагменты служат для поддержания стиля кода проекта. Иногда такие части кода могут быть опасными. Другими словами, дублирующийся код часто указывает на наличие логических ошибок.
Двойная проверка
static void knife_add_single_cut(....)
{
....
if ((lh1->v && lh2->v) && //<=
(lh1->v->v && lh2->v && lh2->v->v) && //<=
(e_base = BM_edge_exists(lh1->v->v, lh2->v->v)))
{
....
return;
}
....
}V501 There are identical sub-expressions 'lh2->v' to the left and to the right of the '&&' operator. editmesh_knife.c 781
Это один из вариантов непродуманного условия. Это, конечно, не ошибка, а лишняя проверка, но это не значит, что код не нуждается в доработке. Условие состоит из нескольких выражений. При этом часть второго выражения полностью соответствует проверке одной из переменных в первом и является лишней. Для исправления требуется убрать лишнюю проверку lh2->v из второго выражения. После этого код станет гораздо наглядней.
Другой пример:
static int edbm_rip_invoke__vert(....)
{
....
if (do_fill) {
if (do_fill) {
....
}
}
....
}V571 Recurring check. The 'if (do_fill)' condition was already verified in line 751. editmesh_rip.c 752
Ещё один вариант логической ошибки. Здесь два абсолютно одинаковых выражения проверяются внутри внешнего и вложенного условия. Повторная проверка всегда будет выдавать одинаковый результат и не имеет смысла. Конечно, данный код никак не влияет на работу программы. Но неизвестно, как будет изменятся код со временем, и лишние проверки могут сбить с толку.
Излишние проверки встречаются далеко не в одном месте проекта. Вот ещё несколько мест, обнаруженных анализатором:
- V571 Recurring check. The 'but' condition was already verified in line 9587. interface_handlers.c 9590
- V571 Recurring check. The '!me->mloopcol' condition was already verified in line 252. paint_vertex.c 253
- V571 Recurring check. The 'constinv == 0' condition was already verified in line 5256. transform_conversions.c 5257
- V571 Recurring check. The 'vlr->v4' condition was already verified in line 4174. convertblender.c 4176
- V571 Recurring check. The 'ibuf == ((void *) 0)' condition was already verified in line 3557. sequencer.c 3559
А третий пример является явным избыточным кодом:
static void writedata_do_write(....)
{
if ((wd == NULL) || wd->error ||
(mem == NULL) || memlen < 1) return;
if (wd->error) return;
....
}V649 There are two 'if' statements with identical conditional expressions. The first 'if' statement contains function return. This means that the second 'if' statement is senseless. Check lines: 331, 332. writefile.c 332
Здесь строка if (wd->error) return; просто лишняя, и функция завершится раньше, чем будет обработано это условие. Поэтому её требуется просто убрать.
Противоположные блоки условия
static int select_less_exec(....)
{
....
if ((lastsel==0)&&(bp->hide==0)&&(bp->f1 & SELECT)){
if (lastsel != 0) sel = 1;
else sel = 0;
....
}
....
}V637 Two opposite conditions were encountered. The second condition is always false. Check lines: 938, 939. editcurve_select.c 938
Из фрагмента верно, что внутри внешнего условного блока расположено дополнительное условие. Вложенное условие противоположно основному и всегда выдаёт одинаковый результат, а переменная sel никогда не получит значение 1. Поэтому достаточно просто прописать sel = 0 без повторной проверки. Хотя, возможно ошибка должна быть исправлена изменением одного из условий. Я не участвую в разработке проекта и мне сложно судить, что здесь к чему.
Избыточные выражения
DerivedMesh *fluidsimModifier_do(....)
{
....
if (!fluidmd || (fluidmd && !fluidmd->fss))
return dm;
....
}V728 An excessive check can be simplified. The '||' operator is surrounded by opposite expressions '!fluidmd' and 'fluidmd'. mod_fluidsim_util.c 528
В рамках одного условия проверяются противоположные значения одной и той же переменной. Такие условия часто встречаются в разных видах и вариациях. Они не наносят вреда при работе программы, но зря усложняют код. Данное выражение можно упростить и привести к следующему виду:
if (!fluidmd || !fluidmd->fss)) ....Похожие места:
- V728 An excessive check can be simplified. The '||' operator is surrounded by opposite expressions '!render_only' and 'render_only'. drawobject.c 4663
- V728 An excessive check can be simplified. The '||' operator is surrounded by opposite expressions '!parent' and 'parent'. kx_scene.cpp 1667
Ещё один вариант такого условия:
void ED_transverts_create_from_obedit(....)
{
....
if ((tipsel && rootsel) || (rootsel)) {....}
....
}V686 A pattern was detected: (rootsel) || ((rootsel) && ...). The expression is excessive or contains a logical error. ed_transverts.c 325
Как и в примере выше, внутри одного выражения проверяется одна и та же переменная дважды. Такое выражение не является ошибочным, но явно перегружено лишней проверкой. Для придания коду большей компактности и наглядности его требуется упростить.
if ((tipsel || rootsel) {....}Подобные ошибки встретились и в других местах проекта.
- V686 A pattern was detected: (!py_b_len) || ((!py_b_len) && ...). The expression is excessive or contains a logical error. aud_pyapi.cpp 864
- V686 A pattern was detected: (xn == 0.0f) || ((xn == 0.0f) && ...). The expression is excessive or contains a logical error. renderdatabase.c 993
- V686 A pattern was detected: (xn == 0.0f) || ((xn == 0.0f) && ...). The expression is excessive or contains a logical error. renderdatabase.c 1115
Повторное присваивание
static bool find_prev_next_keyframes(....)
{
....
do {
aknext = (ActKeyColumn *)BLI_dlrbTree_search_next(
&keys, compare_ak_cfraPtr, &cfranext);
if (aknext) {
if (CFRA == (int)aknext->cfra) {
cfranext = aknext->cfra; //<-
}
else {
if (++nextcount == U.view_frame_keyframes)
donenext = true;
}
cfranext = aknext->cfra; //<-
}
} while ((aknext != NULL) && (donenext == false));
....
}V519 The 'cfranext' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 447, 454. anim_draw.c 454
Присвоение внутри условных блоков не имеет смысла, так как его значение всё равно присваивается заново в конце цикла без какого-либо условия. Сделать вывод, что лишняя строка расположена именно сверху помогает цикл, находящийся в коде следом за приведённым фрагментом. Он отличается только prev переменными и отсутствием этой строки в условии. К тому же, если предположить, что лишняя строка снизу и условие CFRA == (int)aknext->cfra окажется ложным, то цикл превратится в бесконечный. Этот фрагмент явно нуждается в корректировке, но как именно его изменить знают только разработчики проекта.
Лишние или неиспользованные переменные
Подобных фрагментов с инициализированными, но не используемыми в итоге переменными, встретилось очень много. Некоторые из них относятся к примерам логических ошибок и излишних перепроверок, о подобных фрагментах уже много раз говорилось выше. Встречаются также константы, которые вполне возможно должны были изменятся внутри функций. Но в итоге остались лишь в виде проверки, всегда возвращающей одинаковый результат. Пример подобного фрагмента:
static int rule_avoid_collision(....)
{
....
int n, neighbors = 0, nearest = 0; //<=
....
if (ptn && nearest==0) //<=
MEM_freeN(ptn);
return ret;
} V560 A part of conditional expression is always true: nearest == 0. boids.c 361
Остальные фрагменты я приведу просто списком. Возможно, многие из них являются спорными, но на них стоит обратить внимание.
- V560 A part of conditional expression is always true: edit == 0. particle.c 3781
- V560 A part of conditional expression is always true: !error. pointcache.c 154
- V560 A part of conditional expression is always true: !error. pointcache.c 2742
- V560 A part of conditional expression is always false: col. drawobject.c 7803
- V560 A part of conditional expression is always false: !canvas_verts. dynamicpaint.c 4636
- V560 A part of conditional expression is always true: (!leaf). octree.cpp 2513
- V560 A part of conditional expression is always true: (!leaf). octree.cpp 2710
- V560 A part of conditional expression is always false: (1 == i). basicstrokeshaders.cpp 67
- V560 A part of conditional expression is always true: (0 == i). basicstrokeshaders.cpp 69
- V560 A part of conditional expression is always false: (1 == i). basicstrokeshaders.cpp 84
- V560 A part of conditional expression is always true: (0 == i). basicstrokeshaders.cpp 86
- V560 A part of conditional expression is always false: (1 == i). basicstrokeshaders.cpp 155
- V560 A part of conditional expression is always true: (0 == i). basicstrokeshaders.cpp 157
- V560 A part of conditional expression is always true: (!radmod). solver_control.cpp 557
- V560 A part of conditional expression is always true: done != 1. context.c 301
- V560 A part of conditional expression is always true: is_tablet == false. ghost_systemwin32.cpp 665
- V560 A part of conditional expression is always true: mesh >= 0. kx_gameobject.cpp 976
Лишняя очистка списка
int TileManager::gen_tiles(bool sliced)
{
....
state.tiles.clear(); //<=
....
int tile_index = 0;
state.tiles.clear();
state.tiles.resize(num);
....
}V586 The 'clear' function is called twice for deallocation of the same resource. Check lines: 149, 156. tile.cpp 156
В данном случае это может быть просто лишняя строка. Возможно, раньше между двумя очистками списка располагался ещё какой-то код, но в данном случае это ещё один лишний фрагмент, который не несёт пользы и его надо удалить, дабы не загромождать код. Эта строка может быть и следствием того, что в ней должен был очищаться какой-то другой объект, который не видно при беглом осмотре кода. В таком случае фрагмент будет явной ошибкой, которая может привести к неизвестным последствиям для программы.
Очень часто такой, на первый взгляд избыточный, код может привести в итоге к действительно серьёзным ошибкам или поможет избежать их появление в будущем при доработке кода. Именно поэтому стоит обратить внимание на подобные сообщения анализатора, а не отметать их как незначительные.
Интрига
Команда PVS-Studio сейчас активно работает над новым направлением. А я прикрываю тылы, заполняя информационное поле статьями о повторной проверке некоторых открытых проектов. Что это за направление? Не могу сказать. Я только оставлю картинку, которую каждый волен трактовать на свой лад.
Заключение
Анализатор указал достаточно много проблемных мест в проекте. Впрочем, в Blender иногда используется странный стиль кода, и нельзя точно трактовать такие фрагменты как ошибки. Я считаю, опасные ошибки часто возникают из-за опечаток. Именно с их нахождением хорошо справляется анализатор PVS-Studio. Но ошибки, отражённые в статье, это исключительно мнение автора, которое достаточно субъективно. И для полной оценки возможностей анализатора его стоит скачать и попробовать самостоятельно.
Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Alexander Chibisov. PVS-Studio team is about to produce a technical breakthrough, but for now let's recheck Blender.
Комментарии (75)
alexyr
21.07.2016 11:58+1Не факт что первая упомянутая "опечатка" это именно опечатка…
Нужно смотреть логику, возможно сравнивается сама точка с её нормализованным представлением!CaptainTrunky
21.07.2016 12:14+2Все куда проще. В этом коде проверяется близость двух точек. norm — суть эвклидово расстояние между двумя точками. Если точки достаточно близки (норма меньше 1е-06), то что-то там.
dimkss
21.07.2016 13:28Тоже сразу бросилось в глаза.
Randl
21.07.2016 13:42+3Код настолько нечитаемый, что даже с объяснениями не получается понять, что он делает) Ну и неудачный перенос строки играет свою роль. В коде:
if ((iA->getPoint2D() - iA->getPoint2D()).norm() < 1.0e-6) {
а не
if (iA->getPoint2D() - iA->getPoint2D().norm() < 1.0e-6) {
daggert
21.07.2016 12:11+4А можно я погадаю по картинке?) Линукс теперь использует вашу PVS для статанализа ядра? (:
leventov
21.07.2016 12:40Не думали о том, чтобы проверить ClickHouse?
Andrey2008
21.07.2016 12:47Нет. Записали в «посмотреть».
LoadRunner
21.07.2016 13:02+8каждый волен трактовать на свой лад
Лишь бы Мизулина и Милонов не увидели картинку с межвидовым поцелуем. И в радуге всё же 7 цветов, что радует.
Alver
21.07.2016 13:18Вопрос к автору — а вы, случайно, не проверяли код Firefox?
Andrey2008
21.07.2016 14:32+1Как уже успели ответить, да проверяли. Я же хочу дать ссылку для интересующихся, что было, а что нет: Обновляемый список статей, в которых мы рассказываем об ошибках, найденных с помощью PVS-Studio в открытых проектах.
SvyatoslavMC
21.07.2016 14:40Мы проверяли разные проекты Mozilla Foundation, например, Mozilla Filrefox и Mozilla Thunderbird. Я сам пользуюсь этим софтом, поэтому проверял их чаще, чем другие проекты.
Alver
21.07.2016 15:25Если не сложно, можете написать свое мнение по поводу качества кода? Сейчас приходится работать с проектом, делаем браузер на основе именно ФФ. Мое личное мнение, что код в ФФ сильно отстает по качеству от кода Хромиума. Только пока никак не удается уговорить руководство переключиться на использование второго.
Andrey2008
21.07.2016 16:12Думаю будет сложно ответить. Во-первых, этим мы занимались давно. А во-вторых, это слишком субъективно. Надо снимать какие-то метрики и сравнивать, а мы этого не делали.
heleo
21.07.2016 14:42Если это означает, что будет версия под Linux с интеграцией в QtCreator тогда цены вам нет)
EvgeniyRyzhkov
21.07.2016 16:16+6Насчет цены кстати все плохо :-). Мало того, что она «есть», так еще и ого-го!
Про QtCreator. А как Вам кажется должна выглядеть интеграция?VioletGiraffe
21.07.2016 16:53Он поддерживает плагины. В частности, у них сейчас подсветка кода, автодополнение и т. д. (code model) через clang реализована плагином. Правда, не работает ничерта, но это вопросы к разработчикам конкретного плагина, я думаю, а не к системе плагинов :)
EvgeniyRyzhkov
21.07.2016 19:55Ну а вот компилятор вызывается и выдает варнинги в окошко. Этого разве не достаточно (от анализатора)? Зачем еще плагины?
Randl
21.07.2016 20:51Чтобы можно было из IDE прыгать по ошибкам и прямо в IDE исправлять.
EvgeniyRyzhkov
21.07.2016 21:10Ну вот обычный вывод в stdout дает же это?
Randl
21.07.2016 21:17Ну вот сейчас на винде есть Standalone, и есть плагин к Visual Studio. Плагин удобнее
EvgeniyRyzhkov
21.07.2016 21:24+1Эхх… (написал на самом деле другое слово, но смог его заменить на «эхх»)
Сколько раз мне все твердили, что плагин — для дураков, настоящим линуксоидам достаточно command line…
Эххх…Randl
21.07.2016 21:38+2Ну суровым линуксоидам наверное достаточно vi и иксов у них вообще нет. Лично мне удобно когда вся работа с кодом происходит из IDE: контроль версий, профайлинг, дебаггинг ну и статический анализ туда же. Тем более что минимальный анализ IDE делают в реальном времени.
Может я просто не труъ-линуксоид. У меня и винда в дуалбуте есть
Halt
24.07.2016 09:08+1Есть простой критерий — производительность. Да хоть ногами печатайте в стойке на голове, если вам так удобнее. Но только, если вы демонстрируете производительнсоть лучшую, чем показывает сферический разработчик в вакууме.
Однако же, по поводу IDE соглашусь с вами. Задача окружения — свести к минимуму рутиные операции, предоставить весь необходимый инструментарий «на кончиках пальцев» и, в конечном итоге, освободить мозг программиста для работы над задачей.
Неоднократно по работе сталкивался с ситуацией, когда банальнейшие операции коллеги проделывают с таким скрипом и тоской, которые в настроенном окружении выполняются молниеносно и на уровне моторных рефлексов. О какой тогда эффективности можно говорить, если тупо поиск места в коде может занять несколько минут?
Причем, что характерно, подобные индивидуумы обычно утверждают, что «им так удобнее». Но опять же, проблема в итоговой производительности.
P.S.: К слову, «IDE vs vi» звучит не совсем корректно, поскольку vi — это такой же инструмент и можно прокачать его далеко за пределы среднестатистической IDE.Randl
24.07.2016 11:48+1P.S.: К слову, «IDE vs vi» звучит не совсем корректно, поскольку vi — это такой же инструмент и можно прокачать его далеко за пределы среднестатистической IDE.
IDE, я считаю, должно уметь компиляцию, дебаггинг, рефакторинг, кодогенерацию, автоформатирование, статический анализ, контроль версий, подсветку кода, переход к объявлению/реализации. С vim у меня такого не получилось, возможно я не умею его готовить, но рефакторинг, статический анализ и переход к объявлению вроде как принципиально вне IDE невозможен…
Halt
24.07.2016 12:44+2С vim я знаком только шапочно, поэтому не смогу ответить объективно. Наверное стоило это написать в первом комментарии.
Однако я долгое время работал с коллегой, который использовал vim очень активно и расширил его до возможностей, сравнимых с IDE. Навигация по коду там была точно, у vim есть плагин интеграции с clang для семантического поиска и дополнения кода, есть и плагин для отладчика. Про рефакторинг ничего сказать не могу, но и для этого есть плагины.
В KDevelop я работаю уже больше 10 лет. Уверенно редактировать (да даже и просматривать!) код C++ в IDE без семантической информации мне кажется малореальным.
Halt
24.07.2016 09:11Стандартный интерфейс командной строки хорош тем, что его можно интегрировать в любое окружение. По поводу достаточности же скорее не соглашусь (см. выше).
EvgeniyRyzhkov
24.07.2016 11:57+3Плагин к какой системе для PVS-Studio под Linux нам сделать?
Halt
24.07.2016 12:36+1Я думаю это должно решать сообщество :) Лично мне было бы интересно увидеть плагин к KDevelop5, но это только мое мнение.
Я могу только посоветовать вам или сделать стандартный интерфейс, который можно интегрировать в любую IDE или скооперироваться с другой компанией, например JetBrains, и выпустить совместный продукт, который будет работать на любой платформе.
Мне кажется, что даже минимального уровня поддержки в стиле «прогнать анализ на изменившихся файлах», «перейти к коду по клику на ошибке» было бы уже вполне достаточно для более-менее комфортной работы.
Если строки вывода будут сформатированы в стандартном для компиляторов виде, то многие IDE (например KDevelop) сами их подхватят и смогут как навигироваться, так и показывать прогресс проверки в своем статус баре (если проценты будут выводиться в стиле cmake).
Randl
24.07.2016 13:27+2Я считаю, спрашивать одного человека — практически бесполезно. Его ответ будет та IDE, которой он пользуется.
В идеале — сделать удобный интерфейс для подключения к IDE через плагин и опубликовать референсную реализацию для одной из популярных IDE — KDevelop или QtCreator, например. Тот же принцип можно было бы распространить на Windows.
oYASo
22.07.2016 01:24На самом деле, не очень работает под виндой, под никсами бегает очень даже хорошо. Вот почему так происходит — загадка.
nikitasius
21.07.2016 21:03Мало того, что она «есть», так еще и ого-го!
Так на хабреж была статейка, как бесплатно использовать софт, реверсиндиниринг, ассемблер и т.д…EvgeniyRyzhkov
21.07.2016 21:10You are welcome!
nikitasius
22.07.2016 14:17-1mamkaololosha
22.07.2016 14:38-1Отлично. Теперь можно не делать версию под линукс и не убивать время. Можно сконцентрироваться на текущих заказчиках и множить бабло. Если IBM захочет версию под линукс и заплатит миллиард — сделать. А кому нужно будет проверить свою лабораторку за 1 курс — уже знает способ.
nikitasius
22.07.2016 15:12-1Я рад, что я помог вам с решением. Выплатите мне мой гонорар. Не бесплатно же вашей работой руководить.
Andrey2008
22.07.2016 14:52+3Стыдно должно быть. Мы ведь не источаем желчь по поводу этой статьи, не просим её удалить. Лежит себе спокойно, кому нужна — найдёт. Но зачем пиарить то статью? Мы не хотим войны с кряками. Но ведь таким образом Вы можете её спровоцировать. Стыжу всех, кто так делает.
nikitasius
22.07.2016 15:28-4Смещно, когда кто-то пытается кого-то пристыдить в интернете по личным причинам. И с какой такой стати Вы рассуждаете об удалении статьи? Я уверен, *вы просили ее удалить, только:
- статья уже более года
- отличный материал в плане реверса
- отличный материал в плане .NET
- отличный материал в плане asm
У *вас был год на переписывание и проверку вашего кода. Я уверен, что администрация хабра не будет поддаваться на запросы блогов, пусть и платных. Иначе блоги будут выше правил ресурса и будут являть собой цензуру хабра. И последний потеряет часть аудитории. Но, я *вас не буду стыдыть, все компании и, более того, корпопации похожи друг на друга. Вот например на хабре был запрос удалить или переименовать пользователя, а он и ныне там.
В инете полно сайтов, еще больше пользователей. Рано или поздно *вы объявите войну крякам, и я здесь буду ни при чем, а *ваши деньги, только и все. Деньги, которые Вы не захотите терять.
Если бы я писал и продавал софт по аналогии с Вами, я бы озаботился его защитой. Если бы я писал софт по аналогии с Вами, я бы выложил его на github и проект был бы бесплатно допилен и портирован профессионалами своего дела из многих стран. Стыдно Вам должно быть!
*вы, с маленькой буквы, когда идет обсуждения компании в целом или группы лиц.EvgeniyRyzhkov
22.07.2016 15:30-1Если бы я ...
но за школьные каникулы Вы этого сделать не успеете, а через месяц уже опять в школу.nikitasius
22.07.2016 15:35-1но за школьные каникулы Вы этого сделать не успеете, а через месяц уже опять в школу.
Спасибо на добром слове, Евгений Рыжков, генеральный директор ООО «СиПроВер». Сайт: www.viva64.com Почта: evg@viva64.com.
Рад был пообщатся с Вами в вашем блоге. Могу отправить вам список школьной литературы, и после поговорить за жизнь за стаканом парного молока, обмывая новый словарный запас и повышая ваши коммуникационные навыки.
heleo
22.07.2016 00:39Мне бы и лога хватила без встраивания в QtCreator, если уж на то пошло могу и сам попрыгать по коду по логу. А так как написали ниже, в виде плагина вполне удобно, как аналог консоли сборки и вывод ошибок\предупреждений.
Ну а цена… если на фирму брать, то поверьте, бывает ПО под половину лимона деревянных, в котором будут задефайнены свои MIN, MAX и коды ошибок в реализации под Linux будет всегда возвращать код 0, и ничего… покупаем как библиотеки для разработки.EvgeniyRyzhkov
22.07.2016 04:42Вот в то, что бывает ПО «под половину лимона деревянных» мне поверить очень легко, сам таким занимаюсь :-).
Andrey2008
21.07.2016 20:24-1P.S. Нас часто спрашивают, уведомляем ли мы разработчиков. Иногда желание пропадает. :)
Комментарий с баг-трекера:
closed this task as «Invalid»: This is for sure not a bug report — and please do not use our tracker for advertisement of your products.
Да и бог с вами… :)
Halt
21.07.2016 20:43+5Надо сказать, что оформление бага действительно вызывает вопросы. Зачем писать откровенно рекламный текст в багтрекере? Приложили бы выжимку с подозрительными местами и комментариями и никто бы слова не сказал. Ну и ссылку на полную статью, если хочется подробностей.
Здесь же в первую очередь возникает подозрение на спамбота, даже если текст по теме.
Randl
21.07.2016 20:46+3While not a "bug" we do appreciate the writeup. I'll go through the suspicious fragments and fix if needed.
Все таки присоединюсь к предыдущему оратору, оформление бага сомнительное.
mbait
21.07.2016 21:09+1У вас какие-то двойные стандарты. Когда вам постоянно твердили, что нужна версия под Линукс, вы говорили «мало пользователей, убедите нас, покажите ваш проект, всё в индивидуальном порядке». Когда же подходит ваша очередь расписать всё по порядку и оформить отчёт надлежащим образом, вы забиваете и вместо этого, словно с барского плеча, кидаете ссылку на ваш блог. Это выглядит некрасиво. На будущее — в открытых проектах очень цениться взаимное уважение, всем наплевать на ваши регалии.
mamkaololosha
22.07.2016 13:19-4Ну это откровенное хамство. Линукс это целая платформа и на неё либо выходить основательно, либо сжигать тысячу человекочасов и закрываться на тухлой альфе. Темболее на ней будут платить ~1%, а работать должно всё будет на 100%. Тогда проще в Java с их гигантским ентерпрайзом, где без ИДЕ не разобраться. Либо в консольный геимдев, где тоже кода мильены.
mva
24.07.2016 22:17+5Как уже сказали, с таким оформлением бага я бы тоже закрыл его как INVALID даже не вчитываясь. Но, надо отдать должное девелоперам, они не поленились, прошли по ссылке и прошлись по всем расписанным проблемам.
Тем не менее, подход вам, всё же, надо менять. Чтобы потом небыло «говно этот ваш опенсорс, неблагодарные они».
В общем, подписываюсь под словами вышеотписавшихся ораторов.
Saffron
21.07.2016 23:10Вы же вроде опенсорс проектам предоставляете продукт бесплатно. Почему же они отказываются пользоваться статической проверкой?
Andrey2008
21.07.2016 23:26Я не знаю ответа. Впрочем, статическим анализом они пользуются. По крайней мере Coverity с 2007 года: https://scan.coverity.com/projects/blender
sav6622
Ждём… хорошо не с чертом =))
Wesha
Что Вы имеете против нашего чертёнка?
EvgeniyRyzhkov
Претензии все те же — без ошибок не пишут :-). Вот отчет и КДПВ.
Wesha
Кто пишет без ошибок — пусть первый кинет в меня свопом!
Andrey2008
https://habrahabr.ru/company/pvs-studio/blog/277439/