Godot: к вопросу о регулярном использовании статических анализаторов кода

    PVS-Studio and GodotАудитория наших читателей растёт, поэтому мы вновь и вновь пишем статьи, в которых объясняем, как правильно использовать методологию статического анализа кода. Мы считаем очень важным объяснить, что инструменты статического анализа должны использоваться не эпизодически, а регулярно. В очередной раз продемонстрируем это на практическом примере, перепроверив проект Godot.

    Используйте анализаторы регулярно


    Готовясь к выступлению на конференции разработчиков игр, я решил обзавестись новыми примерами интересных ошибок, которые способен выявить инструмент PVS-Studio. С этой целью были проверены несколько игровых движков, одним из которых был Godot. Никаких особенно интересных ошибок для доклада я в нём не нашел, зато мне захотелось написать статью про обыкновенные ошибки. Эти ошибки очень хорошо демонстрируют актуальность регулярного использования инструментов статического анализа кода.

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

    Прошло 3 года. Проект изменился. Анализатор PVS-Studio тоже изменился, и в нём появились новые диагностики. Поэтому нет ничего удивительного, что я легко и быстро смог выписать достаточное количество примеров ошибок для написания этой статьи.

    Однако важно другое. При разработке Godot или любого другого проекта постоянно появляются и исправляются новые ошибки. Ненайденные ошибки «оседают» в коде надолго, и затем многие из них могут быть выявлены при запуске статического анализа кода. Из-за этого иногда возникает ложное ощущение, что статические анализаторы находят только какие-то малоинтересные ошибки в редко используемых участках кода. Конечно, так оно и есть, если использовать анализатор неправильно и запускать его только время от времени, например, незадолго до выпуска релиза.

    Да, мы сами при написании статей выполняем разовые проверки открытых проектов. Но у нас другая цель. Мы хотим продемонстрировать возможности анализатора кода по выявлению дефектов. Эта задача имеет мало общего с повышением качества кода проекта в целом и сокращением издержек, связанных с правкой ошибок.

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

    Теперь перейдём к багам, которые так любят читатели наших публикаций.

    Ошибки из-за Copy-Paste


    Давайте посмотрим, что я заметил, изучая отчёт PVS-Studio. Начну с моей любимой диагностики V501, которая находит ошибки практически в каждом проекте, который мы проверяем :).

    Ошибка N1

    virtual bool can_export(....)
    {
      ....
      if (!exists_export_template("uwp_" + platform_infix + "_debug.zip", &err) ||
          !exists_export_template("uwp_" + platform_infix + "_debug.zip", &err)) {
        valid = false;
        r_missing_templates = true;
      }
      ....
    }

    Предупреждение PVS-Studio: V501 CWE-570 There are identical sub-expressions '!exists_export_template(«uwp_» + platform_infix + "_debug.zip", & err)' to the left and to the right of the '||' operator. export.cpp 1135

    Классическая Copy-Paste ошибка. Вызов функции скопирован, но не отредактирован. Имя второго обрабатываемого файла должно заканчиваться на "_release.zip".

    Ошибки N2, N3

    static String dump_node_code(SL::Node *p_node, int p_level) {
      ....
      if (bnode->statements[i]->type == SL::Node::TYPE_CONTROL_FLOW ||
          bnode->statements[i]->type == SL::Node::TYPE_CONTROL_FLOW) {
        code += scode; //use directly
      } else {
        code += _mktab(p_level) + scode + ";\n";
      }
      ....
    }

    Предупреждение PVS-Studio: V501 CWE-570 There are identical sub-expressions 'bnode->statements[i]->type == SL::Node::TYPE_CONTROL_FLOW' to the left and to the right of the '||' operator. test_shader_lang.cpp 183

    void EditorSpinSlider::_notification(int p_what) {
      if (p_what == MainLoop::NOTIFICATION_WM_FOCUS_OUT ||
          p_what == MainLoop::NOTIFICATION_WM_FOCUS_OUT) {
        if (grabbing_spinner) {
          Input::get_singleton()->set_mouse_mode(Input::MOUSE_MODE_VISIBLE);
          grabbing_spinner = false;
          grabbing_spinner_attempt = false;
        }
      }
      ....
    }

    Предупреждение PVS-Studio: V501 CWE-570 There are identical sub-expressions 'p_what == MainLoop::NOTIFICATION_WM_FOCUS_OUT' to the left and to the right of the '||' operator. editor_spin_slider.cpp 157

    Думаю, ошибки хорошо видны и не требуют каких-либо пояснений. Точно такой же классический Copy-Paste, как и в первом случае.

    Ошибка N4

    String SoftBody::get_configuration_warning() const {
      ....
      Transform t = get_transform();
      if ((ABS(t.basis.get_axis(0).length() - 1.0) > 0.05 ||
           ABS(t.basis.get_axis(1).length() - 1.0) > 0.05 ||
           ABS(t.basis.get_axis(0).length() - 1.0) > 0.05)) {
        if (!warning.empty())
      ....
    }

    Предупреждение PVS-Studio: V501 CWE-570 There are identical sub-expressions to the left and to the right of the '||' operator. soft_body.cpp 399

    Здесь первая строка была скопирована дважды. Но номер оси координат был изменён только во второй строке. А третью строчку отредактировать забыли. Это не что иное, как "Эффект последней строки".

    Примечание. На данный момент, помимо «Эффекта последней строки», мною сделаны следующие интересные наблюдения: "Самая опасная функция в мире С/С++", "Зло живёт в функциях сравнения". И сейчас я сделаю анонс новой статьи, написанием которой я планирую заняться в ближайшее время. Рабочее название «0, 1, 2». Должно получиться интересно и поучительно. Приглашаю подписываться на один из каналов, чтобы не пропустить: twitter, vk.com, Instagram, telegram и «олдскульный» rss.

    Ошибка N5

    void ScrollContainer::_notification(int p_what) {
      ....
      if (h_scroll->is_visible_in_tree() && h_scroll->get_parent() == this)
        size.y -= h_scroll->get_minimum_size().y;
    
      if (v_scroll->is_visible_in_tree() && v_scroll->get_parent() == this)
        size.x -= h_scroll->get_minimum_size().x;
      ....
    }

    Предупреждение PVS-Studio: V778 CWE-682 Two similar code fragments were found. Perhaps, this is a typo and 'v_scroll' variable should be used instead of 'h_scroll'. scroll_container.cpp 249

    По поводу этого фрагмента кода у меня нет полной уверенности, что здесь есть ошибка. Однако я согласен с анализатором, что второй блок выглядит очень подозрительно. Скорее всего, код писался с помощью Copy-Paste и во втором блоке текста забыли заменить h_scroll на v_scroll.

    Вероятно, код должен быть таким:

    if (h_scroll->is_visible_in_tree() && h_scroll->get_parent() == this)
      size.y -= h_scroll->get_minimum_size().y;
    
    if (v_scroll->is_visible_in_tree() && v_scroll->get_parent() == this)
      size.x -= v_scroll->get_minimum_size().x;

    Ошибка N6

    Ещё один случай, когда был скопирован и неудачно изменён достаточно большой фрагмент кода. Строчка с ошибкой помечена мною комментарием "// <=".

    void ShaderGLES2::bind_uniforms() {
      ....
      const Map<uint32_t, Variant>::Element *E = uniform_defaults.front();
    
      while (E) {
        int idx = E->key();
        int location = version->uniform_location[idx];
    
        if (location < 0) {
          E = E->next();
          continue;
        }
    
        Variant v;
        v = E->value();
        _set_uniform_variant(location, v);
        E = E->next();
      }
    
      const Map<uint32_t, CameraMatrix>::Element *C = uniform_cameras.front();
    
      while (C) {
        int idx = E->key();                                  // <=
        int location = version->uniform_location[idx];
    
        if (location < 0) {
          C = C->next();
          continue;
        }
    
        glUniformMatrix4fv(location, 1, GL_FALSE, &(C->get().matrix[0][0]));
        C = C->next();
      }
    
      uniforms_dirty = false;
    }

    Предупреждение PVS-Studio: V522 CWE-476 Dereferencing of the null pointer 'E' might take place. shader_gles2.cpp 102

    Ошибка выявлена косвенным образом. Благодаря анализу потока данных PVS-Studio выявил, что указатель E может быть нулевым в момент его разыменования.

    Ошибка заключается в том, что в скопированном фрагменте кода забыли заменить в одном месте E на C. Из-за этой ошибки функция работает очень странным образом и делает непонятные вещи.

    Опечатки


    Ошибка N7

    Программистам, пишущим не на языке C или C++, сложно представить, что можно допустить опечатку, написав вместо звёздочки '*' запятую ',', и при этом код будет компилироваться. Однако это так.

    LRESULT OS_Windows::WndProc(....) {
      ....
      BITMAPINFO bmi;
      ZeroMemory(&bmi, sizeof(BITMAPINFO));
      bmi.bmiHeader.biSize = sizeof(BITMAPINFOHEADER);
      bmi.bmiHeader.biWidth = dib_size.x;
      bmi.bmiHeader.biHeight = dib_size.y;
      bmi.bmiHeader.biPlanes = 1;
      bmi.bmiHeader.biBitCount = 32;
      bmi.bmiHeader.biCompression = BI_RGB;
      bmi.bmiHeader.biSizeImage = dib_size.x, dib_size.y * 4;
      ....
    }

    Предупреждение PVS-Studio: V521 CWE-480 Such expressions using the ',' operator are dangerous. Make sure the expression is correct. os_windows.cpp 776

    Переменной bmi.bmiHeader.biSizeImage присваивается значение переменной dib_size.x. Далее выполняется оператор запятая ',', чей приоритет ниже, чем у оператора '='. Результат же выражения dib_size.y * 4 никак не используется.

    Вместо запятой в выражении должен использоваться оператор умножения '*'. Во-первых, такое выражение будет иметь смысл. Во-вторых, ниже есть похожий, но уже корректный вариант инициализации такой же переменной:

    bmi.bmiHeader.biSizeImage = dib_size.x * dib_size.y * 4;

    Ошибки N8, N9

    void Variant::set(....) {
      ....
      int idx = p_index;
      if (idx < 0)
        idx += 4;
      if (idx >= 0 || idx < 4) {
        Color *v = reinterpret_cast<Color *>(_data._mem);
        (*v)[idx] = p_value;
        valid = true;
      }
      ....
    }

    Предупреждение PVS-Studio: V547 CWE-571 Expression 'idx >= 0 || idx < 4' is always true. variant_op.cpp 2152

    Любой индекс будет считаться корректным. Чтобы исправить ошибку, надо заменить оператор || на &&:

    if (idx >= 0 && idx < 4) {

    Эта логическая ошибка возникла, скорее всего, по невнимательности, поэтому я склонен отнести её к опечаткам.

    Точно такую же ошибку можно наблюдать в этом же файле чуть ниже. Виной размножения ошибки, по всей видимости, является Copy-Paste.

    Размноженная ошибка: V547 CWE-571 Expression 'idx >= 0 || idx < 4' is always true. variant_op.cpp 2527

    Ошибка N10

    WTF?

    Пример ошибки, от которой так и хочется воскликнуть: WTF?!

    void AnimationNodeBlendSpace1D::add_blend_point(
      const Ref<AnimationRootNode> &p_node, float p_position, int p_at_index)
    {
      ERR_FAIL_COND(blend_points_used >= MAX_BLEND_POINTS);
      ERR_FAIL_COND(p_node.is_null());
    
      ERR_FAIL_COND(p_at_index < -1 || p_at_index > blend_points_used);
    
      if (p_at_index == -1 || p_at_index == blend_points_used) {
        p_at_index = blend_points_used;
      } else {
        for (int i = blend_points_used - 1; i > p_at_index; i++) {
          blend_points[i] = blend_points[i - 1];
        }
      }
      ....
    }

    Предупреждение PVS-Studio: V621 CWE-835 Consider inspecting the 'for' operator. It's possible that the loop will be executed incorrectly or won't be executed at all. animation_blend_space_1d.cpp 113

    Обратите внимание на условие остановки цикла: i > p_at_index. Оно всегда истинно, так как переменная i инициализируется значением blend_points_used — 1. При этом из двух более ранних проверок следует, что blend_points_used > p_at_index.

    Условие может стать ложным, только когда возникнет переполнение знаковой переменной i, что является неопределённым поведением. Более того, до переполнения не дойдёт, так как намного раньше произойдёт выход за границу массива.

    Перед нами, на мой взгляд, красивейшая опечатка, когда вместо '<' написали '>'. Да, у меня извращённое представление о красоте ошибок :).

    Корректный цикл:

    for (int i = blend_points_used - 1; i < p_at_index; i++) {

    Ошибка N11

    Ещё один не менее яркий случай опечатки в условии цикла.

    void AnimationNodeStateMachineEditor::_state_machine_pos_draw() {
      ....
      int idx = -1;
      for (int i = 0; node_rects.size(); i++) {
        if (node_rects[i].node_name == playback->get_current_node()) {
          idx = i;
          break;
        }
      }
      ....
    }

    Предупреждение PVS-Studio: V693 CWE-835 Consider inspecting conditional expression of the loop. It is possible that 'i < X.size()' should be used instead of 'X.size()'. animation_state_machine_editor.cpp 852

    Может произойти выход за границу массива, так как значение i увеличивается бесконтрольно. Безопасный код:

    for (int i = 0; i < node_rects.size(); i++) {

    Ошибка N12

    GDScriptDataType GDScriptCompiler::_gdtype_from_datatype(
      const GDScriptParser::DataType &p_datatype) const
    {
      ....
      switch (p_datatype.kind) {
         ....
        case GDScriptParser::DataType::NATIVE: {
          result.kind = GDScriptDataType::NATIVE;
          result.native_type = p_datatype.native_type;
        } break;
        case GDScriptParser::DataType::SCRIPT: {
          result.kind = GDScriptDataType::SCRIPT;
          result.script_type = p_datatype.script_type;
          result.native_type = result.script_type->get_instance_base_type();
        }
        case GDScriptParser::DataType::GDSCRIPT: {
          result.kind = GDScriptDataType::GDSCRIPT;
          result.script_type = p_datatype.script_type;
          result.native_type = result.script_type->get_instance_base_type();
        } break;
      ....
    }

    Предупреждение PVS-Studio: V796 CWE-484 It is possible that 'break' statement is missing in switch statement. gdscript_compiler.cpp 135

    Случайно забыли написать оператор break. Поэтому при попадании в case GDScriptParser::DataType::SCRIPT переменным будут присвоены значения, как будто это case GDScriptParser::DataType::GDSCRIPT.

    Ошибка N13

    Следующую ошибку можно классифицировать как Copy-Paste. Однако я не уверен, что столь короткая строка была скопирована. Так что будем считать это простой опечаткой при наборе текста.

    void CPUParticles::_particles_process(float p_delta) {
      ....
      if (flags[FLAG_DISABLE_Z]) {
        p.velocity.z = 0.0;
        p.velocity.z = 0.0;
      }
      ....
    }

    Предупреждение PVS-Studio: V519 CWE-563 The 'p.velocity.z' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 664, 665. cpu_particles.cpp 665

    Два раза происходит присваивание одной и той же переменной. Ниже можно увидеть вот такой фрагмент кода:

    if (flags[FLAG_DISABLE_Z]) {
      p.velocity.z = 0.0;
      p.transform.origin.z = 0.0;
    }

    Скорее всего, для первого случая должно было быть написано так же.

    Ошибка N14

    bool AtlasTexture::is_pixel_opaque(int p_x, int p_y) const {
      if (atlas.is_valid()) {
        return atlas->is_pixel_opaque(
          p_x + region.position.x + margin.position.x,
          p_x + region.position.y + margin.position.y
        );
      }
      return true;
    }

    Предупреждение PVS-Studio: V751 Parameter 'p_y' is not used inside function body. texture.cpp 1085

    Фрагмент из описания диагностики V751:

    Анализатор обнаружил подозрительную функцию, один из параметров которой ни разу не используется. При этом другой его параметр используется несколько раз, что, возможно, свидетельствует о наличии ошибки.

    Как видите, это действительно так, и это очень подозрительно. Переменная p_x используется дважды, а p_y не используется. Скорее всего, должно быть написано:

    return atlas->is_pixel_opaque(
      p_x + region.position.x + margin.position.x,
      p_y + region.position.y + margin.position.y
    );

    Кстати, в исходном коде вызов функции написан в одну строчку. Из-за этого ошибку сложнее заметить. Если бы автор кода написал фактические аргументы в столбик, как я сделал это в статье, то ошибка сразу бросилась бы в глаза. Не забывайте, что табличное форматирование весьма полезно и позволяет избежать множества опечаток. См. главу «Выравнивайте однотипный код „таблицей“ в статье „Главный вопрос программирования, рефакторинга и всего такого“.

    Ошибка N15

    bool SpriteFramesEditor::can_drop_data_fw(....) const {
      ....
      Vector<String> files = d["files"];
    
      if (files.size() == 0)
        return false;
    
      for (int i = 0; i < files.size(); i++) {
        String file = files[0];
        String ftype = EditorFileSystem::get_singleton()->get_file_type(file);
    
        if (!ClassDB::is_parent_class(ftype, "Texture")) {
          return false;
        }
      }
      ....
    }

    Предупреждение PVS-Studio: V767 Suspicious access to element of 'files' array by a constant index inside a loop. sprite_frames_editor_plugin.cpp 602

    В цикле обрабатывается один и тот же файл на всех итерациях цикла. Опечатка здесь:

    String file = files[0];

    Должно быть:

    String file = files[i];

    Прочие ошибки


    Ошибка N16

    CSGBrush *CSGBox::_build_brush() {
      ....
      for (int i = 0; i < 6; i++) {
        ....
        if (i < 3)
          face_points[j][(i + k) % 3] = v[k] * (i >= 3 ? -1 : 1);
        else
          face_points[3 - j][(i + k) % 3] = v[k] * (i >= 3 ? -1 : 1);
        ....
      }
      ....
    }

    Анализатор PVS-Studio выдаёт сразу два срабатывания на этот код:

    • V547 CWE-570 Expression 'i >= 3' is always false. csg_shape.cpp 939
    • V547 CWE-571 Expression 'i >= 3' is always true. csg_shape.cpp 941

    И действительно, вот этот тернарный оператор в обоих выражениях выглядит очень странно:

    i >= 3 ? -1 : 1 

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

    for (int i = 0; i < 6; i++) {
      ....
      if (i < 3)
        face_points[j][(i + k) % 3] = v[k];
      else
        face_points[3 - j][(i + k) % 3] = -v[k];
      ....
    }

    Я могу быть неправ, и код надо исправить совсем другим способом.

    Ошибка N17

    Ошибок типа V595 почти не нашлось, хотя обычно их с избытком обнаруживается в любом проекте. Видимо, эти ошибки были исправлены после предыдущей проверки, и затем ошибки этого типа почти не появлялись. Я увидел только несколько ложных срабатываний и одну ошибку.

    bool CanvasItemEditor::_get_bone_shape(....) {
      ....
      Node2D *from_node = Object::cast_to<Node2D>(
                            ObjectDB::get_instance(bone->key().from));
      ....
      if (!from_node->is_inside_tree())
        return false; //may have been removed
      if (!from_node)
        return false;
      ....
    }

    Предупреждение PVS-Studio: V595 CWE-476 The 'from_node' pointer was utilized before it was verified against nullptr. Check lines: 565, 567. canvas_item_editor_plugin.cpp 565

    Указатель from_node вначале разыменовывается для вызова функции is_inside_tree и только затем проверятся на равенство nullptr. Проверки следует поменять местами:

    if (!from_node)
      return false;
    if (!from_node->is_inside_tree())
      return false; //may have been removed

    Ошибка N18
    enum JoystickList {
      ....
      JOY_AXIS_MAX = 10,
      ....
    };
    
    static const char *_axes[] = {
      "Left Stick X",
      "Left Stick Y",
      "Right Stick X",
      "Right Stick Y",
      "",
      "",
      "L2",
      "R2"
    };
    
    int InputDefault::get_joy_axis_index_from_string(String p_axis) {
      for (int i = 0; i < JOY_AXIS_MAX; i++) {
        if (p_axis == _axes[i]) {
          return i;
        }
      }
      ERR_FAIL_V(-1);
    }

    Предупреждение PVS-Studio: V557 CWE-125 Array overrun is possible. The value of 'i' index could reach 9. input_default.cpp 1119

    Массив _axes состоит из восьми элементов. При этом константа JOY_AXIS_MAX, задающая количество итераций цикла, равна 10. Получается, что в цикле происходит выход за границу массива.

    Ошибка N19

    И последняя очень странная функция, которая, видимо, предназначена для тестирования чего-то. Функция длинная, поэтому я приведу её в виде картинки (нажмите на картинку для увеличения).

    Очень странная функция



    Предупреждение PVS-Studio: V779 CWE-561 Unreachable code detected. It is possible that an error is present. test_math.cpp 457

    В функции встречается несколько безусловных операторов return. На картинке я отметил их красными овалами. Такое впечатление, что в эту функцию собрали несколько разных юнит-тестов, но забыли удалить лишние return NULL. В результате функция не проверяет то, что должна проверять. Почти всё тело функции состоит из недостижимого кода.

    Возможно, конечно, это какая-то хитрая задумка. Но мне кажется, это получилось случайно и код следует исправить.

    На этом давайте закончим. Наверняка, если внимательно всмотреться в отчёт анализатора, можно найти и другие ошибки. Однако даже выписанного с лихвой хватило для написания статьи. Дальше уже станет скучно и мне, и читателю :).

    Заключение


    В статье описаны ошибки, которых бы не существовало, если бы код регулярно анализировался с помощью PVS-Studio. Однако, что ещё более важно, используя анализ регулярно, можно было бы сразу найти и устранить множество других ошибок. Более развернуто эту мысль описал мой коллега в заметке: „Философия статического анализа кода: у нас 100 программистов, анализатор нашел мало ошибок, он бесполезен?“. Очень рекомендую потратить 10 минут на прочтение этой короткой, но очень важной статьи.

    Спасибо за внимание. Приглашаю всех скачать и попробовать статический анализ PVS-Studio для проверки ваших собственных проектов.



    Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Andrey Karpov. Godot: On Regular Use of Static Analyzers.
    PVS-Studio
    775,00
    Static Code Analysis for C, C++, C# and Java
    Поделиться публикацией

    Комментарии 19

      +4
      Ошибка №10 — вот не вижу ошибки, серьезно. p_at_index передается снаружи, может принимать значения от -1 до blend_points_used, где предположительно blend_points_used есть длина массива blend_points (аллоцированная может быть больше). Как следствие, если p_at_index== blend_points_used, выполняется вставка элемента в конец массива, а если где-то в середине, то выполняется сдвиг элементов по массиву в сторону больших индексов до позиции p_at_index, в которую ниже выполняется вставка. И таки да, в этом случае цикл for имеет право не выполниться ни разу…

      А, нашел — переменную i нужно инициализировать новым значением последнего индекса массива, которое равно blend_points_used, а там стоит blend_points_used-1. В этом случае цикл отработает минимум один раз, сдвинув последний элемент в позицию [blends_points_used] и освободив место для вставки. Таки CWE в виде потери ссылки на последний объект в списке, хи-хи. Респект! И правильно, цикл должен идти в обратную сторону, т.е. i--. Действительно, вместе взятое получается WTF.
        –1
        del copy
          +2

          Если смысл анализатора в предотвращении ошибок ещё до их первого попадания в коммит и раньше, а не выявлении застарелых, то возможно было бы наглядно провести другой эксперимент? Взять 10 последних багфиксов, которые сделали сами авторы без анализатора и показать, могли ли они быть предотвращены с помощью анализатора на раннем этапе?

            +1
            Можно, но это сложно и очень нудно. Если хотите, попробуйте провести такой эксперимент сами и поймёте, почему мы таких статей не пишем. :)
              0
              Понимаю, но я не продаю статический анализатор ;). Придется покупателям и дальше верить Вам на слово, когда Вы говорите, что анализатор наиболее полезен для предотвращения ошибок на раннем этапе.
                +3
                Прошло немало времени, пока проверялся проект, пока я готовился к докладу, пока писалась статья, пока она переводилась. Как результат по крайней мере 2 ошибки уже были исправлены. А их могло бы и вообще не быть…
                  +1
                  А разве есть техническая разница в поиске ошибок на раннем этапе и разовых проверках? По-моему, очевидно, что всё нашлось бы при первом применении. Тем более, поведение анализатора детерминировано на одном и том же коде. Нет никаких оснований «брать на веру», весь материал состоит из пруфов)
                    0
                    Есть, это разные ошибки, о чем говорится в статье. Те ошибки, которые находятся при разовых проверках — это, в основном, несущественные ошибки, с которыми пользователи и тестеры не столкнулись. А вот найдутся ли те ошибки, ради которых потом, с кучей усилий, приходится выпускать багфиксы — не вполне очевидно.
                    Пруфы в этой статье касаются только первого типа, то есть ошибок не найденных ни тестами ни пользователями, а значит незначительных.
                    Совершенно не очевидно, что технически (с точки зрения анализатора) «важные» (те, что потом будут обнаружены тестами и пользователями) ошибки и те что в статье — одинаковы.
                    Вот какой процент багфиксов по-вашему можно избежать применяя PVS-studio?
                      0
                      то есть ошибок не найденных ни тестами ни пользователями, а значит незначительных
                      Очень странное заключение…
                        +1
                        Вы оба правы. Действительно существую ошибки, которые находятся в неиспользуемом коде. Толку от их нахождения мало, вот только сложно со стороны сказать, является ошибка таковой или нет.

                        Однако, то что ошибка долго не обнаруживается, не значит, что она несущественна. Возможно, это противный гейзенбаг, который всем досаждает, но который пользователю трудно повторить и описать. В результате, пользователь начинает с меньшим интересом относиться к приложению или вообще просто тихо перестаёт им пользоваться. Ещё один вариант не проявляющей себя ошибки — уязвимость. Их надо искать и править, но пользователь с ними не сталкивается.
                          0
                          то что ошибка долго не обнаруживается, не значит, что она несущественна.
                          Не значит, но это очень сильно скоррелированные параметры. Большая часть застарелых ошибок, как вы и сами говорите, несущественны.
                          Их надо искать и править, но пользователь с ними не сталкивается.
                          сталкивается тестер, сталкивается пользователь, когда уязвимость кто-то начинает эксплуатировать.
                          +2
                          это не мое заключение, а заключение автора статьи:
                          Смысл статического анализа кода не в том, чтобы найти застаревшие ошибки. Эти старые ошибки обычно незначительны, иначе бы они мешали пользователям и их бы уже нашли и исправили.

                          И дальше идет анализ именно таких застаревших ошибок.
                          При чем автор и сам осознает проблему:
                          возникает ложное ощущение, что статические анализаторы находят только какие-то малоинтересные ошибки в редко используемых участках кода. Конечно, так оно и есть, если использовать анализатор неправильно и запускать его только время от времени

                          Но тем не менее доказательств ложности этого ощущения не приводится, что и понятно, ведь это сложно и нудно.
                0

                Зима уж близится, а всё-таки, как там прогресс по анализатору Java?
                Насчет C++ давно понятно, что отстрелить себе ногу там можно тысячами изощрённых способов, как без анализатора, так и с ним вместе.

                  +2
                  Постепенно рассылаем beta-версии PVS-Studio Java на пробу, работаем с отзывами.
                  +2
                  Кстати говоря, кроме вашей проверки в 2015 году, его ещё относительно недавно в 2017 проверяли
                    0
                    Противостояние багов и программистов будет вечным, если не обмазаться инструментами контроля качества. Про Google Chromium уже 13 статей написано…
                    0
                    Наверное, задам очень банальный вопрос, но вы отправляете авторам open-source проектов патчи с исправлениями после написания таких статей? Было бы очень здорово!

                  Только полноправные пользователи могут оставлять комментарии. Войдите, пожалуйста.

                  Самое читаемое