# Нарезка кода: что скрывает OrcaSlicer Как не пропустить опасные участки кода при ревью? Можно воспользоваться инструментами статического анализа\. Возьмём для примера OrcaSlicer — популярную программу, которая подготавливает 3D\-модель к печати\. Заглянем внутрь и посмотрим, какие сюрпризы нас ждут\. ![1389_OrcaSlicer_ru/image1.png](https://import.viva64.com/docx/blog/1389_OrcaSlicer_ru/image1.png) ## На повестке дня На просторах GitHub есть интересный проект для подготовки деталей к печати на 3D\-принтере\. Он позволяет преобразовать 3D\-модель в управляющие команды для послойного создания объекта\. [OrcaSlicer](https://github.com/OrcaSlicer/OrcaSlicer) — это один из самых популярных бесплатных слайсеров с открытым исходным кодом\. Проект довольно большой, а основной язык разработки — C\+\+, именно поэтому он и попал в мои руки\. Сегодня предлагаю изучить его изнутри и найти интересные места в коде\. > \*\*Примечание\*\*\\\. Данный контент создан с целью популяризации технологии статического анализа и не пытается оскорбить авторов OrcaSlicer\\\. > > Эта статья только показывает, что даже в работающем проекте могут быть ошибки\\\. Их пропускают при ревью или тестах, они годами накапливаются, что в итоге может приводить к проблемам в продукте\\\. Однако использование дополнительных средств проверки, например статических анализаторов, позволяет обнаружить ошибки ещё на этапе разработки\\\. > > Если вы всё ещё сомневаетесь в пользе таких инструментов, то рекомендую прочитать статью "\[Причины внедрить в процесс разработки статический анализатор кода PVS\\\-Studio\]\(https://pvs\-studio\.ru/ru/blog/posts/0687/\)"\\\. ## Ныряем в глубину Анализ проекта проводился на основе этого [коммита](https://github.com/OrcaSlicer/OrcaSlicer/commit/a167702038496b97e8495cc79f0c62660063ac87) с помощью плагина [PVS\-Studio](https://pvs-studio.ru/ru/) для Visual Studio 2022\. В статье были собраны не все, а только самые интересные, на мой взгляд, предупреждения анализатора\. > \*\*Примечание\\\.\*\* Для упрощения чтения фрагменты кода были отформатированы, а некоторые скрыты за комментариями \`// \.\.\.\.\`\\\. **Пример N1** Предупреждение PVS\-Studio: [V655](https://pvs-studio.ru/ru/docs/warnings/v655/) The strings were concatenated but are not utilized\. Consider inspecting the 'message \+ "\\n\\nApplication will close\."' expression\. [GUI\_App\.cpp 7170](https://github.com/OrcaSlicer/OrcaSlicer/blob/a167702038496b97e8495cc79f0c62660063ac87/src/slic3r/GUI/GUI_App.cpp#L7170) ```cpp bool GUI_App::load_language(wxString language, bool initial) { // .... if (!wxLocale::IsAvailable(locale_language_info->Language)) { // Loading the language dictionary failed. wxString message = "Switching Orca Slicer to language " + requested_language_code + " failed."; #if !defined(_WIN32) && !defined(__APPLE__) // likely some linux system message += "\nYou may need to reconfigure the missing locales, " " likely by running the \"locale-gen\" " "and \"dpkg-reconfigure locales\" commands.\n"; #endif if (initial) message + "\n\nApplication will close."; // <= wxMessageBox(message, "Orca Slicer - Switching language failed", wxOK | wxICON_ERROR); if (initial) std::exit(EXIT_FAILURE); else return false; } // .... } ``` Опечатка, которую не заметил разработчик и которую пропустили при код\-ревью\. Вместо оператора `+=` был использован `+`, и в результате строковый литерал не добавляется к переменной `message`\. Код компилируется, но не имеет никакого смысла\. Это предупреждение напомнило мне [ошибку из qbEngine](https://pvs-studio.ru/ru/blog/posts/cpp/1119/#ID717E26CA54), когда в макросе `#else` забыли дописать `#`\. При взгляде на этот код, возможно, у вас тоже возникнут мысли: "Ну нет\.\.\. Это смешно, я такое точно никогда не напишу"\. Но это просто человеческий фактор\. Это код, который был заложен четыре года назад и до сих пор не был исправлен\. При этом место, где инициализируется строка `message`, [изменяли](https://github.com/OrcaSlicer/OrcaSlicer/commit/8168d0a4e088748dde3d843fd1aa77eed245f528#diff-8432157ace185a6e4c63a53f838d80cd1bbae25fdd3829b94a54fc5ecf5b6dabL6443-R6466) пару месяцев назад\. **Пример N2** Предупреждения PVS\-Studio: [V1047](https://pvs-studio.ru/ru/docs/warnings/v1047/) Lifetime of the lambda is greater than lifetime of the local variable 'do\_stop' captured by reference\. [FillBedJob\.cpp 250](https://github.com/OrcaSlicer/OrcaSlicer/blob/a167702038496b97e8495cc79f0c62660063ac87/src/slic3r/GUI/Jobs/FillBedJob.cpp#L250) ```cpp void FillBedJob::process(Ctl &ctl) { // .... bool do_stop = false; // .... params.on_packed = [&do_stop] (const ArrangePolygon &ap) { do_stop = ap.bed_idx > 0 && ap.priority == 0; }; // .... } ``` Лямбда\-выражение захватывает локальную переменную `do_stop` по ссылке, а затем сохраняется в `params.on_packed`\. При этом `do_stop` уничтожается при выходе из метода `process`, так как заканчивается время жизни локального объекта\. Если лямбда будет вызвана после выхода из этой функции\-члена, произойдёт обращение к разрушенному объекту, и поведение в этой ситуации не определено\. Можно было бы сделать захват по значению, но в этой лямбде происходит перезапись переменной `do_stop`, а значит такой вариант не подходит\. Поэтому можно сделать `do_stop` членом класса `FillBedJob`\. Ниже по коду аналогичная ситуация происходит ещё два раза: * V1047 Lifetime of the lambda is greater than lifetime of the local variable 'statustxt' captured by reference\. [FillBedJob\.cpp 245](https://github.com/OrcaSlicer/OrcaSlicer/blob/a167702038496b97e8495cc79f0c62660063ac87/src/slic3r/GUI/Jobs/FillBedJob.cpp#L245) * V1047 Lifetime of the lambda is greater than lifetime of the local variable 'do\_stop' captured by reference\. [FillBedJob\.cpp 241](https://github.com/OrcaSlicer/OrcaSlicer/blob/a167702038496b97e8495cc79f0c62660063ac87/src/slic3r/GUI/Jobs/FillBedJob.cpp#L241) **Пример N3** Предупреждение PVS\-Studio: [V768](https://pvs-studio.ru/ru/docs/warnings/v768/) The enumeration constant 'roPortrait' is used as a variable of a Boolean\-type\. [RasterBase\.hpp 81](https://github.com/OrcaSlicer/OrcaSlicer/blob/a167702038496b97e8495cc79f0c62660063ac87/src/libslic3r/SLA/RasterBase.hpp#L81) ```cpp class RasterBase { public: enum Orientation { roLandscape, roPortrait }; struct Trafo { bool mirror_x = false, mirror_y = false, flipXY = false; TMirroring get_mirror() const { return { (roPortrait ? !mirror_x : mirror_x), mirror_y}; } // <= // .... }; // .... } ``` Анализатор обнаружил использование элемента перечисления в тернарном операторе в качестве первого операнда\. Константа `roPortrait` имеет значение `1`, поэтому условие всегда будет истинно\. Вероятно, здесь должна использоваться другая переменная\. А возможно, нужно было добавить дополнительный параметр, чтобы сравнить его с `roPortrait`, как это сделано в конструкторе: ```cpp Trafo(Orientation o = roLandscape, const TMirroring &mirror = NoMirror) // XY flipping implicitly does an X mirror : mirror_x(o == roPortrait ? !mirror[0] : mirror[0]) // <= , mirror_y(!mirror[1]) // Makes raster origin to be top left corner , flipXY(o == roPortrait) {} ``` **Пример N4** Предупреждение PVS\-Studio: [V781](https://pvs-studio.ru/ru/docs/warnings/v781/) The value of the 'm\_load\_slot\_index' variable is checked after it was used\. Perhaps there is a mistake in program logic\. Check lines: 2152, 2155\. [AMSItem\.cpp 2152](https://github.com/OrcaSlicer/OrcaSlicer/blob/a167702038496b97e8495cc79f0c62660063ac87/src/slic3r/GUI/Widgets/AMSItem.cpp#L2152) ```cpp void AMSRoadUpPart::doRender(wxDC& dc) { // .... dc.SetPen(wxPen(_get_diff_clr(this, m_amsinfo.cans[m_load_slot_index].material_colour), 4, wxPENSTYLE_SOLID)); auto temp = m_amsinfo; if (m_load_step != AMSPassRoadSTEP::AMS_ROAD_STEP_NONE){ if (m_amsinfo.cans.size() <= m_load_slot_index || m_load_slot_index < 0){ BOOST_LOG_TRIVIAL(trace) << "up road render error"; return; } // .... } ``` Такие ошибки очень интересны тем, что кажется невозможным здесь ошибиться\. Вроде бы простое место, но довольно легко запутаться\. Если посмотреть в [список](https://pvs-studio.ru/ru/blog/examples/v781/) найденных диагностическим правилом V781 ошибок, то можно понять, что даже разработчики довольно крупных проектов часто их допускают\. Но в чём проблема? Выход за границы массива\. Сначала индекс `m_load_slot_index` используется для доступа к элементу вектора `m_amsinfo.cans`, и только после этого проверяется, не выходит ли итератор за границы и не отрицательный ли он\. Вроде проверка есть, но она срабатывает слишком поздно\. Чтобы код работал корректно, можно поставить проверку выше или обработать такую ситуацию другим способом\. Аналогичное предупреждение: * V781 The value of the 'i' index is checked after it was used\. Perhaps there is a mistake in program logic\. ViewerImpl\.cpp 56 **Пример N5** Предупреждение PVS\-Studio: [V783](https://pvs-studio.ru/ru/docs/warnings/v783/) Iterator 'it' is checked after it was used\. Perhaps there is a mistake in program logic\. Check lines: 3159, 3160\. [Selection\.cpp 3160](https://github.com/OrcaSlicer/OrcaSlicer/blob/a167702038496b97e8495cc79f0c62660063ac87/src/slic3r/GUI/Selection.cpp#L3160) ```cpp void Selection::ensure_not_below_bed() { // .... if (is_any_volume()) { for (unsigned int i : m_list) { GLVolume& volume = *(*m_volumes)[i]; const std::pair instance = std::make_pair(volume.object_idx(), volume.instance_idx()); InstancesToZMap::const_iterator it = instances_max_z.find(instance); const double z_shift = SINKING_MIN_Z_THRESHOLD - it->second; if (it != instances_max_z.end() && z_shift > 0.0) volume.set_volume_offset(Z, volume.get_volume_offset(Z) + z_shift); } } // .... } ``` В этом же фрагменте есть ещё одна проверка, и она тоже почему\-то стоит после использования итератора\. Для вычисления `z_shift` разыменовывается итератор `it`, а строчкой ниже он проверяется на неравенство итератору `end()`\. Если элемент не был найден, то поведение при разыменовании результирующего итератора не определено\. Исправленный вариант кода: ```cpp if (it != instances_max_z.end()) { const double z_shift = SINKING_MIN_Z_THRESHOLD - it->second; if (z_shift > 0.0) volume.set_volume_offset(Z, volume.get_volume_offset(Z) + z_shift); } ``` **Пример N6** Предупреждения PVS\-Studio: [V595](https://pvs-studio.ru/ru/docs/warnings/v595/) The 'part\_plate' pointer was utilized before it was verified against nullptr\. Check lines: 498, 525\. [ObjectDataViewModel\.cpp 498](https://github.com/OrcaSlicer/OrcaSlicer/blob/a167702038496b97e8495cc79f0c62660063ac87/src/slic3r/GUI/ObjectDataViewModel.cpp#L498) [V522](https://pvs-studio.ru/ru/docs/warnings/v522/) Dereferencing of the null pointer 'part\_plate' might take place\. The null pointer is passed into 'AddPlate' function\. Inspect the first argument\. Check lines: 497, 535\. [ObjectDataViewModel\.cpp 497](https://github.com/OrcaSlicer/OrcaSlicer/blob/a167702038496b97e8495cc79f0c62660063ac87/src/slic3r/GUI/ObjectDataViewModel.cpp#L497) ```cpp wxDataViewItem ObjectDataViewModel::AddOutsidePlate(bool refresh) { wxDataViewItem plate_item = AddPlate(nullptr, _L("Outside")); // <= m_plate_outside = (ObjectDataViewModelNode*)plate_item.GetID(); return plate_item; } // .... wxDataViewItem ObjectDataViewModel::AddPlate (PartPlate* part_plate, wxString name, bool refresh) { int plate_idx = part_plate ? part_plate->get_index() : -1; // <= 1 wxString plate_name = name; if (name.empty()) { plate_name = _L("Plate"); plate_name += wxString::Format(" %d", plate_idx + 1); if (!part_plate->get_plate_name().empty()) // <= 2 { plate_name += wxString(" (", wxConvUTF8) + from_u8(part_plate->get_plate_name()) + wxString(")", wxConvUTF8); } } // .... for (int obj_idx = 0; obj_idx < m_objects.size(); obj_idx++) { auto obj_node = m_objects[obj_idx]; if (part_plate && part_plate->contain_instance_totally(obj_idx, 0)){// <= 3 ReparentObject(plate_node, obj_node); } } return plate_item; } ``` Фрагмент большой, поэтому давайте по порядку\. В функции\-члене `AddOutsidePlate` происходит вызов функции\-члена `AddPlate`, где первым аргументом передают `nullptr`\. Соответствующий параметр — `part_plate` — проверяется на валидность в двух из трёх случаев использования\. Если в одном месте проверки нет, то компилятор может посчитать, что и остальные проверки не нужны, и просто уберёт их в целях оптимизации\. Можно исправить код, добавив недостающую проверку\. Интересный факт: если посмотреть на [историю изменений](https://github.com/OrcaSlicer/OrcaSlicer/commit/14cb2449c6b4b60ea9587e8f94ae32d554b10a8f#diff-b491cdc4033e90ac53f2919d673d1a51e1e673ca4143cf0b9887905e537f22f1L458-R463), можно увидеть, что изначально там [стоял](https://github.com/OrcaSlicer/OrcaSlicer/commit/14cb2449c6b4b60ea9587e8f94ae32d554b10a8f#diff-b491cdc4033e90ac53f2919d673d1a51e1e673ca4143cf0b9887905e537f22f1L461) тернарный оператор, но потом код переписали\. **Пример N7** Предупреждение PVS\-Studio: [V797](https://pvs-studio.ru/ru/docs/warnings/v797/) The 'find' function is used as if it returned a bool type\. The return value of the function should probably be compared with std::string::npos\. [HintNotification\.cpp 175](https://github.com/OrcaSlicer/OrcaSlicer/blob/a167702038496b97e8495cc79f0c62660063ac87/src/slic3r/GUI/HintNotification.cpp#L175) ```cpp TagCheckResult tag_check_material(const std::string& tag) { if (const GUI::Tab* tab = wxGetApp().get_tab(Preset::Type::TYPE_FILAMENT)) { // search PrintConfig filament_type to find if allowed tag if (wxGetApp().app_config->get("filament_type").find(tag)) { const Preset& preset = tab->m_presets->get_edited_preset(); const auto* opt = preset.config.opt("filament_type"); if (opt->values[0] == tag) return TagCheckAffirmative; return TagCheckNegative; } return TagCheckNotCompatible; } return TagCheckNotCompatible; } ``` В коде есть комментарий, который сообщает, что необходимо найти нужный тег\. Ниже находится условие, в котором и происходит поиск тега\. Но проблема в том, что оно работает не так, как задумывалось: * если тег будет найден в начале, то `find` вернёт `0`, что будет сконвертировано в `false`; * если тег будет найден в середине, то `find` вернёт положительное число, что будет сконвертировано в `true`; * если тег не будет найден, то `find` вернёт `std::string::npos`, что будет сконвертировано в `true`\. В итоге условие проверяет не то, что тег найден, а то, что такой тег не находится в начале строки\. Исправить код можно следующим образом: ```cpp if (wxGetApp().app_config->get("filament_type").find(tag) != std::string::npos) ``` **Пример N8** Предупреждение PVS\-Studio: [V714](https://pvs-studio.ru/ru/docs/warnings/v714/) Variable 'face' is not passed into foreach loop by a reference, but its value is changed inside of the loop\. [MeshBoolean\.cpp 153](https://github.com/OrcaSlicer/OrcaSlicer/blob/a167702038496b97e8495cc79f0c62660063ac87/src/libslic3r/MeshBoolean.cpp#L153) ```cpp template void triangle_mesh_to_cgal(const TriangleMesh& M, _Mesh& out) { // .... // Number the faces because 'orient_to_bound_a_volume' // needs a face <--> index map unsigned index = 0; for (auto face : out.faces()) // <= face = CGAL::SM_Face_index(index++); // .... } ``` Анализатор сообщает о странном диапазонном цикле: переменная цикла `face` объявляется как копия очередного элемента диапазона `out.faces()` и при этом модифицируется на каждой итерации\. По всей видимости, код должен был изменить элементы диапазона `out.faces()`\. Если элементы какого\-то диапазона нужно изменить, то переменная цикла должна быть объявлена как lvalue\-ссылка: ```cpp for (auto &face : out.faces()) face = CGAL::SM_Face_index(index++); ``` Давайте на всякий случай убедимся, что элементы диапазона `out.faces()` не являются аналогами `std::reference_wrapper`\.
Спойлер: нет, не являются ```cpp template class Iterator_range : public std::pair { public: I begin() const { return this->first; } I end() const { return this->second; } template Iterator_range make_range(const T& b, const T&e) { return Iterator_range(b,e); } // .... }; template class SM_Index { // .... public: typedef boost::uint32_t size_type; // .... protected: size_type idx_; }; class SM_Face_index : public SM_Index { /* .... */ }; typedef SM_Face_index Face_index; template class Surface_mesh { // .... private: template class Index_iterator : public boost::iterator_facade< Index_iterator, Index_, std::random_access_iterator_tag, Index_ > { // .... private: friend class boost::iterator_core_access; // .... Index_ dereference() const { return hnd_; } Index_ hnd_; const Surface_mesh* mesh_; }; // .... public: typedef Index_iterator Face_iterator; typedef Iterator_range Face_range; Face_iterator faces_begin() const { return Face_iterator(Face_index(0), this); } /// End iterator for faces. Face_iterator faces_end() const { return Face_iterator(Face_index(num_faces()), this); } Face_range faces() const { return make_range(faces_begin(), faces_end()); } // .... }; ``` Кода много, но попробуем разобраться\. В шаблоне функции `triangle_mesh_to_cgal` параметр `out` имеет шаблонный тип `_Mesh`\. Чаще всего аргументами шаблона функции выступают специализации шаблона класса `Surface_mesh`\. Внутри него в приватной части определяется шаблон итератора `Index_iterator`, который наследуется от [`boost::iterator_facade`](https://www.boost.org/doc/libs/latest/libs/iterator/doc/facade-and-adaptor.html)\. По сути, этот шаблон класса упрощает имплементацию итераторов\. Давайте посмотрим на его шаблонные параметры: ```cpp template < class Derived // The derived iterator type being constructed , class Value , class CategoryOrTraversal , class Reference = Value& , class Difference = std::ptrdiff_t > class iterator_facade ``` Четвёртый шаблонный параметр задаёт тип [`reference`](https://cppreference.com/cpp/iterator/iterator_traits), т\. е\. то, что будет возвращать итератор при разыменовании\. Если он не указан явно, то считается, что `reference` — это `Value &`\. Интересуемый нас шаблон класса `Index_iterator` передаёт четвёртым аргументом свой шаблонный тип, т\. е\. при разыменовании итератора будет возвращаться копия `Index_`\. В качестве шаблонного типа `Index_` выступает тип `Face_index`\. Это псевдоним типа `SM_Face_index`, в котором не определяются никакие поля и который наследуется от шаблона класса `SM_Index`\. В самом шаблоне класса `SM_Index` определяется одно поле `idx` типа `boost::uint32_t`, т\. е\. встроенного интегрального типа\.
Я затрудняюсь ответить, каким именно должен был быть код в этой ситуации, т\. к\. изменение типа переменной `face` на `auto &` не выглядит как исправление\. Более того, код должен перестать компилироваться, т\. к\. lvalue\-ссылка не может быть привязана к prvalue\-объекту\. При разыменовании итератора `Face_iterator` возвращается копия объекта `Face_index`\. Чтобы модифицировать что\-то внутри диапазона `out.faces()`, нужно, чтобы итератор возвращал ссылку и был каким\-то образом ассоциирован с внутренностями `Surface_mesh`\. Тогда бы диапазонный цикл `for` работал корректно\. Однако сам `Surface_mesh` с итератором находится в third\-party библиотеке, и я не уверена, что там можно делать какие\-либо изменения\. **Пример N9** Предупреждение PVS\-Studio: [V609](https://pvs-studio.ru/ru/docs/warnings/v609/) Divide by zero\. Denominator range \[0x0\.\.0x7FFFFFFFFFFFFFFF\]\. [TreeSupport3D\.cpp 811](https://github.com/OrcaSlicer/OrcaSlicer/blob/a167702038496b97e8495cc79f0c62660063ac87/src/libslic3r/Support/TreeSupport3D.cpp#L811) ```cpp [[nodiscard]] static Polygons safe_offset_inc(....) { if (distance == 0) return do_final_difference ? diff(ret, collision_trimmed()) : union_(ret); if (safe_step_size < 0 || last_step_offset_without_check < 0) { BOOST_LOG_TRIVIAL(warning) << "Offset increase got invalid parameter!"; tree_supports_show_error( "Negative offset distance... How did you manage this ?"sv, true); return do_final_difference ? diff(ret, collision_trimmed()) : union_(ret); } coord_t step_size = safe_step_size; int steps = distance > last_step_offset_without_check ? (distance - last_step_offset_without_check) / step_size : 0; // .... } ``` Анализатор сообщает, что произошло деление на ноль\. Чтобы понять, в чём дело, нужно начать с конца\. Переменная `step_size` равна `safe_step_size`, а значит, деление на ноль произойдёт, если `safe_step_size == 0`\. В начале функции есть проверка `safe_step_size < 0`\. Она отлавливает только отрицательные значения, но не проверяет равенство нулю\. Значит, раннего выхода не случится, если `safe_step_size == 0`, и из\-за этого произойдёт деление на ноль\. Эта проблема встречается довольно часто и связана с использованием констант 0, 1, 2\. Мы обнаружили такую закономерность и даже написали об этом статью "[Ноль, один, два, Фредди заберёт тебя](https://pvs-studio.ru/ru/blog/posts/cpp/0713/)"\. Другие предупреждения V609: * V609 Divide by zero\. Denominator range \[0\.\.2\]\. GCode\.hpp 303 **Пример N10** Предупреждение PVS\-Studio: [V593](https://pvs-studio.ru/ru/docs/warnings/v593/) Consider reviewing the expression of the 'A \= B < C' kind\. The expression is calculated as following: 'A \= \(B < C\)'\. [PerimeterGenerator\.cpp 1057](https://github.com/OrcaSlicer/OrcaSlicer/blob/a167702038496b97e8495cc79f0c62660063ac87/src/libslic3r/PerimeterGenerator.cpp#L1057) ```cpp template class LinesDistancer { using Floating = typename std::conditional< std::is_floating_point::value, Scalar, double>::type; template Floating distance_from_lines(/*.... */) //.... } std::tuple, Polygons> generate_extra_perimeters_over_overhangs(....) { // .... for (size_t i = 0; i < overhang_region.front().polyline.size(); i++) { Point p = overhang_region.front().polyline.points[i].to_point(); if (double d = lower_layer_aabb_tree.distance_from_lines(p) < min_dist) { min_dist = d; min_dist_idx = i; } } // .... } ``` Хотели написать красивый код, но не учли приоритет операций\. Оператор `<` выполняется раньше присваивания, поэтому переменная `d` получает результат сравнения, то есть значение типа `bool`, а не `double`\. И в итоге в переменную `min_dist` записывается либо `0`, либо `1`, а не реальное расстояние\. Исправить код можно так: ```cpp double d = lower_layer_aabb_tree.distance_from_lines(p); if (d < min_dist) ``` Или так, если компилятор поддерживает инструкцию `if` с инициализатором \(C\+\+17\): ```cpp if (double d = lower_layer_aabb_tree.distance_from_lines(p); d < min_dist) ``` **Пример N11** Предупреждение PVS\-Studio: [V766](https://pvs-studio.ru/ru/docs/warnings/v766/) An item with the same key '"open"' has already been added\. [DialogButtons\.hpp 69](https://github.com/OrcaSlicer/OrcaSlicer/blob/a167702038496b97e8495cc79f0c62660063ac87/src/slic3r/GUI/Widgets/DialogButtons.hpp#L68-L69) ```cpp class DialogButtons : public wxPanel { private: const std::map m_standardIDs = { // Choice {"ok" , wxID_OK}, {"yes" , wxID_YES}, {"apply" , wxID_APPLY}, {"confirm" , wxID_APPLY}, // no id for confirm, reusing wxID_APPLY {"no" , wxID_NO}, {"cancel" , wxID_CANCEL}, // Action {"open" , wxID_PRINT}, // <= {"open" , wxID_OPEN}, // <= //.... } ``` В инициализацию `std::map` добавили два элемента с ключом `open`\. Контейнер не может содержать несколько одинаковых элементов, поэтому второй элемент не будет добавлен\. Если посмотреть на значения, то становится понятно, что при копировании забыли заменить ключ для [`wxID_PRINT`](https://docs.wxwidgets.org/3.2/defs_8h.html#ac66d0a09761e7d86b2ac0b2e0c6a8cbba4d4fdc32f37cc92c05d7a4ff2bb12ee2)\. Из\-за этого будет использоваться не то значение\. Скорее всего, исправить код можно так: ```cpp {"print" , wxID_PRINT}, {"open" , wxID_OPEN}, ```
Другие предупреждения V766 V766 An item with the same key '"use\_firmware\_retraction"' has already been added\. Print\.cpp 229 V766 An item with the same key '"filament\_notes"' has already been added\. Print\.cpp 237 V766 An item with the same key '"nozzle\_volume"' has already been added\. PrintConfig\.cpp 8201 V766 An item with the same key '"inner\-outer\-inner wall/infill"' has already been added\. PrintConfig\.cpp 272
**Пример N12** Предупреждение PVS\-Studio: [V734](https://pvs-studio.ru/ru/docs/warnings/v734/) An excessive expression\. Examine the substrings "plus" and "xplus"\. [QidiPrinterAgent\.cpp 375](https://github.com/OrcaSlicer/OrcaSlicer/blob/a167702038496b97e8495cc79f0c62660063ac87/src/slic3r/Utils/QidiPrinterAgent.cpp#L375) ```cpp std::string QidiPrinterAgent::infer_series_id (const std::string& model_id, const std::string& dev_name) { // .... if ( ( key.find("xplus") != std::string::npos || key.find("plus") != std::string::npos) && key.find("4") != std::string::npos) { return "0"; } return ""; } ``` Анализатор обнаружил лишний вызов в условии\. Если `key` содержит подстроку `xplus`, то она содержит и `plus`\. Второе условие `find("plus")` избыточно, т\. к\. оно будет истинно всегда, когда истинно первое\. Достаточно проверить только `plus`: ```cpp if (key.find("plus") != std::string::npos && key.find("4") != std::string::npos) ``` А ещё сам код выглядит не очень эффективно: строку перебирают три раза\. **Пример N13** Предупреждение PVS\-Studio: [V583](https://pvs-studio.ru/ru/docs/warnings/v583/) The '?:' operator, regardless of its conditional expression, always returns one and the same value: wxEmptyString\. [Search\.cpp 96](https://github.com/OrcaSlicer/OrcaSlicer/blob/a167702038496b97e8495cc79f0c62660063ac87/src/slic3r/GUI/Search.cpp#L96) ```cpp static Option create_option (const std::string &opt_key, const wxString &label, Preset::Type type, const GroupAndCategory &gc) { wxString suffix; wxString suffix_local; if (gc.category == "Machine limits") { //suffix = opt_key.back() == '1' ? L("Stealth") : L("Normal"); suffix = opt_key.back() == '1' ? wxEmptyString : wxEmptyString; suffix_local = " " + _(suffix); suffix = " " + suffix; } // .... } ``` Опечатка в тернарном операторе: оба варианта возвращают `wxEmptyString`\. Условие не влияет на результат\. По закомментированной выше строке видно, что ранее здесь была другая логика, но в текущем виде код работает совершенно по\-другому\. Точно такой же код был скопирован и в другое место: [V583](https://pvs-studio.ru/ru/docs/warnings/v583/) The '?:' operator, regardless of its conditional expression, always returns one and the same value: wxEmptyString\. [Search\.cpp 348](https://github.com/OrcaSlicer/OrcaSlicer/blob/a167702038496b97e8495cc79f0c62660063ac87/src/slic3r/GUI/Search.cpp#L348) **Пример N14** Предупреждение PVS\-Studio: [V670](https://pvs-studio.ru/ru/docs/warnings/v670/) The uninitialized class member 'm\_result' is used to initialize the 'm\_options\_z\_corrector' member\. Remember that members are initialized in the order of their declarations inside a class\. [GCodeProcessor\.cpp 1671](https://github.com/OrcaSlicer/OrcaSlicer/blob/a167702038496b97e8495cc79f0c62660063ac87/src/libslic3r/GCode/GCodeProcessor.cpp#L1671) ```cpp class OptionsZCorrector { GCodeProcessorResult& m_result; public: explicit OptionsZCorrector(GCodeProcessorResult& result) : m_result(result) { } // .... } class GCodeProcessor { // .... OptionsZCorrector m_options_z_corrector; // line: 811 // .... GCodeProcessorResult m_result; // line: 843 // .... } GCodeProcessor::GCodeProcessor() : m_options_z_corrector(m_result) { // .... } ``` Использование неинициализированной переменной в списке инициализации конструктора\. Поля класса инициализируются в порядке их объявления, и в этом коде `m_options_z_corrector` инициализируется [раньше](https://github.com/OrcaSlicer/OrcaSlicer/blob/a167702038496b97e8495cc79f0c62660063ac87/src/libslic3r/GCode/GCodeProcessor.hpp#L811-L843), чем `m_result`\. Сейчас код работает корректно, потому что место в памяти под объект выделяется раньше, чем происходит инициализация\. В таком случае есть возможность взять адрес и прочитать его, что и было сделано в конструкторе `OptionsZCorrector`\. Но если в нём будет использоваться переменная `m_result`, то возникнет неопределённое поведение\. Чтобы код работал корректно, нужно поменять местами объявления полей в классе так, чтобы `m_result` было выше, чем `m_options_z_corrector`\. ## Смотрим в будущее Если вы тоже хотите минимизировать ошибки в своих проектах, сделайте статический анализ частью [регулярного процесса](https://pvs-studio.ru/ru/pvs-studio/algoritm/), а помочь с этим может [PVS\-Studio](https://pvs-studio.ru/ru/pvs-studio/download/)\. [Ознакомиться с ценами](https://pvs-studio.ru/ru/order/), [запросить пробную версию](https://pvs-studio.ru/ru/pvs-studio/try-free/) или [узнать условия бесплатного лицензирования](https://pvs-studio.ru/ru/blog/posts/1286/) вы можете в соответствующих разделах\. Смотрите в будущее своего проекта и ловите ошибки как можно раньше :\)