# Code slicing: What lies inside OrcaSlicer How can you avoid overlooking dangerous code parts during reviews? You can use static analysis tools\. Let's take as an example OrcaSlicer, a popular slicing software designed to prepare 3D models for printing\. We'll take a peek under its hood and see what surprises await us\. ![1389_OrcaSlicer/image1.png](https://import.viva64.com/docx/blog/1389_OrcaSlicer/image1.png) ## About the project There's a cool project on GitHub that helps prepare 3D models for printing\. It can convert a 3D model into machine instructions for creating an object layer by layer\. [OrcaSlicer](https://github.com/OrcaSlicer/OrcaSlicer) is one of the most popular free open\-source slicers\. The project is quite large, and its primary development language is C\+\+, which is exactly why it caught my attention\. Today, let's take a look under the hood and explore some of the code's highlights\. > \*\*Note\*\*\\\. The purpose of this content is to promote static analysis technology, not to offend the authors of OrcaSlicer\\\. > > The article simply shows that even functioning projects can contain bugs\\\. Developers may overlook issues during code reviews or testing, and they accumulate over the years, which can ultimately lead to defects in the product\\\. However, using additional tools, such as static analyzers, helps developers detect bugs before they reach production\\\. > > If you still doubt the value of such tools, I recommend reading the article "\[Why you should choose the PVS\\\-Studio static analyzer to integrate into your development process\]\(https://pvs\-studio\.com/en/blog/posts/0687/\)\\\." ## Let's dive in I analyzed this project [commit](https://github.com/OrcaSlicer/OrcaSlicer/commit/a167702038496b97e8495cc79f0c62660063ac87) using [PVS\-Studio](https://pvs-studio.com/en/) plugin for Visual Studio 2022\. The article doesn't cover every warning the analyzer issued, only the ones I found the most interesting\. > \*\*Note\\\.\*\* To improve readability, the code snippets have been formatted, and some have been hidden behind comments marked with \`// \.\.\.\.\`\\\. **Example N1** PVS\-Studio warning: [V655](https://pvs-studio.com/en/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; } // .... } ``` Here's a typo that developers missed during code review\. The code uses `+` instead of `+=`, so the string literal never gets appended to the `message` variable\. The code compiles, but it makes no sense\. This warning reminded me of an [error we caught in qbEngine](https://pvs-studio.com/en/blog/posts/cpp/1119/#ID717E26CA54), where devs forgot to add the `#` character in the `#else` macro\. When looking at this code, one might think: "No way\! I'd never write something like this\." It's simply a mistake a human can do, though\. This code has been in the project for four years, yet it still hasn't been fixed\. At the same time, the fragment where the `message` string is initialized was [modified](https://github.com/OrcaSlicer/OrcaSlicer/commit/8168d0a4e088748dde3d843fd1aa77eed245f528#diff-8432157ace185a6e4c63a53f838d80cd1bbae25fdd3829b94a54fc5ecf5b6dabL6443-R6466) just a couple of months ago\. **Example N2** PVS\-Studio warning: [V1047](https://pvs-studio.com/en/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; }; // .... } ``` The lambda expression captures the `do_stop` local variable by reference and then stores it in `params.on_packed`\. However, `do_stop` is destroyed when the `process` method exits because the lifetime of the local object ends\. If the lambda runs after the member function returns, it will access a destroyed object, resulting in undefined behavior\. Capturing by value would solve this issue, but the lambda modifies the `do_stop` variable, so this approach won't work\. Instead, `do_stop` can be turned into a member of the `FillBedJob` class\. This pattern repeats two more times in the code: * 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) **Example N3** PVS\-Studio warning: [V768](https://pvs-studio.com/en/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}; } // <= // .... }; // .... } ``` The analyzer detected an enum value being used as the first operand in a ternary operator\. The `roPortrait` constant has a value of `1`, so the condition is always true\. Most likely, the developers should've used another variable here\. Alternatively, an additional parameter may have been needed to compare it with `roPortrait`, as done in the constructor: ```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) {} ``` **Example N4** PVS\-Studio warning: [V781](https://pvs-studio.com/en/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; } // .... } ``` These errors are especially intriguing because they seem impossible to make at first glance\. The case may seem simple, but it's actually quite confusing\. If we look through the [list of issues](https://pvs-studio.com/en/blog/examples/v781/) the V781 diagnostic rule detected, we can see that even developers working on fairly large projects make these mistakes quite often\. What's the problem, though? It's array index out of bounds\. First, the `m_load_slot_index` index is used to access an element of the `m_amsinfo.cans` vector, only then it's checked whether the iterator is out of bounds or negative\. The check exists, but it happens too late\. We can fix the code by placing the check earlier or handling this case differently\. The same warning: * V781 The value of the 'i' index is checked after it was used\. Perhaps there is a mistake in program logic\. ViewerImpl\.cpp 56 **Example N5** PVS\-Studio warning: [V783](https://pvs-studio.com/en/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); } } // .... } ``` This fragment contains another check that also appears after the iterator is used\. The`it` iterator is dereferenced to calculate `z_shift`, and the next line contains a check whether it differs from the `end()` iterator\. If the element isn't found, dereferencing the resulting iterator leads to undefined behavior\. The fixed code: ```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); } ``` **Example N6** PVS\-Studio warnings: [V595](https://pvs-studio.com/en/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.com/en/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; } ``` This is a long snippet, so let's break it down step by step\. The `AddOutsidePlate` member function calls `AddPlate` and passes `nullptr` as the first argument\. The `part_plate` parameter is checked two out of the three times where it's used\. If a check is missing somewhere, the compiler may assume that the other checks are unnecessary as well and remove them for optimization purposes\. We can fix the code by adding the missing check\. Here's a fun fact: if we look at the [commit history](https://github.com/OrcaSlicer/OrcaSlicer/commit/14cb2449c6b4b60ea9587e8f94ae32d554b10a8f#diff-b491cdc4033e90ac53f2919d673d1a51e1e673ca4143cf0b9887905e537f22f1L458-R463), we can see that [there was](https://github.com/OrcaSlicer/OrcaSlicer/commit/14cb2449c6b4b60ea9587e8f94ae32d554b10a8f#diff-b491cdc4033e90ac53f2919d673d1a51e1e673ca4143cf0b9887905e537f22f1L461) originally a ternary operator there, but the code was later rewritten\. **Example N7** PVS\-Studio warning: [V797](https://pvs-studio.com/en/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; } ``` The code contains a comment explaining that a tag needs to be found\. Below is the condition where the tag is searched for\. However, it doesn't work as intended: * if the tag is found at the beginning, `find` returns `0`, which is converted to `false`; * if the tag is found somewhere in the middle, `find` returns a positive number, which is converted to `true`; * if the tag is not found, `find` returns `std::string::npos`, which is also converted to `true`\. As a result, the condition doesn't check whether the tag was found\. Instead, it checks whether the tag isn't located at the beginning of the string\. The fix may look like this: ```cpp if (wxGetApp().app_config->get("filament_type").find(tag) != std::string::npos) ``` **Example N8** PVS\-Studio warning: [V714](https://pvs-studio.com/en/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++); // .... } ``` The analyzer reports a suspicious range\-based loop: the `face` loop variable is declared as a copy of the current element from the `out.faces()` range, but the code modifies it on every iteration\. Most likely, the code was intended to modify elements inside the `out.faces()` range\. If you need to modify elements within a range, declare the loop variable as an lvalue reference: ```cpp for (auto &face : out.faces()) face = CGAL::SM_Face_index(index++); ``` Let's also make sure that the elements in the `out.faces()` range aren't the same as `std::reference_wrapper`\.
Spoiler: no, they aren't ```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()); } // .... }; ``` There's a lot of code here, so let's try to break it down\. In the `triangle_mesh_to_cgal` function template, the out parameter has the `_Mesh` template type\. Most often, function template arguments are specializations of the `Surface_mesh` class template\. In the private class section, the template of the `Index_iterator` iterator, which inherits from `[boost::iterator_facade](https://www.boost.org/doc/libs/latest/libs/iterator/doc/facade-and-adaptor.html)`, is defined\. Essentially, this class template simplifies iterator implementation\. Let's take a look at its template parameters: ```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 ``` The fourth template parameter specifies the `[reference](https://cppreference.com/cpp/iterator/iterator_traits)` type, which is the value the iterator returns when dereferenced\. If it isn't specified explicitly, `reference` defaults to `Value &`\. The `Index_iterator` class template we're interested in passes its own template type as the fourth argument\. This means dereferencing the iterator returns a copy of `Index_`\. The `Index_` template parameter is of the `Face_index` type\. This is an alias of the `SM_Face_index` type, which doesn't define any data members and inherits from the `SM_Index` class template\. The `SM_Index` class template defines a single `idx` data member of the `boost::uint32_t` type, which is a built\-in integral type\.
I find it difficult to say what the code should've looked like here, since changing the `face` variable type to `auto &` doesn't seem like a valid fix\. Moreover, the code should stop compiling because an lvalue reference can't bind to a prvalue object\. When `Face_iterator` is dereferenced, it returns a copy of the `Face_index` object\. To modify anything within the `out.faces()` range, the iterator would need to return a reference and somehow be connected to the `Surface_mesh` internal data\. In that case, the range\-based `for` loop would work correctly\. However, `Surface_mesh` and its iterator come from a third\-party library, and I doubt we can make any changes there\. **Example N9** PVS\-Studio warning: [V609](https://pvs-studio.com/en/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; // .... } ``` The analyzer reports a division by zero\. To understand what's happening, we'll start from the end\. The `step_size` variable equals `safe_step_size`, which means the division by zero occurs if `safe_step_size == 0`\. At the beginning of the function, there's a `safe_step_size < 0` check, which catches only negative values and doesn't check for zero\. This means the function doesn't return early when `safe_step_size == 0`, which eventually leads to a division by zero\. This issue appears quite often and is related to using the constants 0, 1, and 2\. We noticed this pattern and even wrote an article about it: "[Zero, one, two, Freddy's coming for you](https://pvs-studio.com/en/blog/posts/cpp/0713/)\." Other V609 warnings: * V609 Divide by zero\. Denominator range \[0\.\.2\]\. GCode\.hpp 303 **Example N10** PVS\-Studio warning: [V593](https://pvs-studio.com/en/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; } } // .... } ``` The devs wanted to write clean code, but overlooked operator precedence\. The `<` operator has higher precedence than assignment, so the `d` variable receives the result of the comparison—a `bool` value instead of `double`\. As a result, `min_dist` gets either `0` or `1` instead of the actual distance\. Here's how we can fix this: ```cpp double d = lower_layer_aabb_tree.distance_from_lines(p); if (d < min_dist) ``` Alternatively, we can fix it like this if the compiler supports the `if` statement with an initializer \(C\+\+17\): ```cpp if (double d = lower_layer_aabb_tree.distance_from_lines(p); d < min_dist) ``` **Example N11** PVS\-Studio warning: [V766](https://pvs-studio.com/en/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}, // <= //.... } ``` Two elements with the `open` key were added to the initialization of `std::map`\. A container can't store multiple elements with the same key, so the second element won't be added\. Looking at the values, we can see that the key for `[wxID_PRINT](https://docs.wxwidgets.org/3.2/defs_8h.html#ac66d0a09761e7d86b2ac0b2e0c6a8cbba4d4fdc32f37cc92c05d7a4ff2bb12ee2)` wasn't updated after copying\. As a result, the wrong value will be used\. The fix may look like this: ```cpp {"print" , wxID_PRINT}, {"open" , wxID_OPEN}, ```
Other V766 warnings 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
**Example N12** PVS\-Studio warning: [V734](https://pvs-studio.com/en/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 ""; } ``` The analyzer detected an unnecessary call in the condition\. If `key` contains the `xplus` substring, it also contains `plus`\. The second `find("plus")` check is redundant because it will always evaluate to true whenever the first one does\. Checking `plus` is enough: ```cpp if (key.find("plus") != std::string::npos && key.find("4") != std::string::npos) ``` The code also looks rather inefficient: the string is iterated over three times\. **Example N13** PVS\-Studio warning: [V583](https://pvs-studio.com/en/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; } // .... } ``` There's a typo in the ternary operator: both branches return `wxEmptyString`\. The condition doesn't affect the result\. The commented\-out line above indicates that the code previously had different logic, but it works differently in its current form\. The exact same code was copied to another snippet as well: [V583](https://pvs-studio.com/en/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) **Example N14** PVS\-Studio warning: [V670](https://pvs-studio.com/en/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) { // .... } ``` An uninitialized variable is used in the constructor initialization list\. Class data members initialize in the order they're declared, and in this code, `m_options_z_corrector` initializes [before](https://github.com/OrcaSlicer/OrcaSlicer/blob/a167702038496b97e8495cc79f0c62660063ac87/src/libslic3r/GCode/GCodeProcessor.hpp#L811-L843) `m_result`\. Currently, the code works correctly because memory for the object gets allocated before initialization begins\. In this case, it's possible to retrieve the address and read it, which is exactly what happens in the `OptionsZCorrector` constructor\. However, if the constructor starts using the `m_result` variable, it will lead to undefined behavior\. To fix the code, the data member declarations in the class should be reordered so that `m_result` appears before `m_options_z_corrector`\. ## Looking ahead If you wish to minimize errors in your projects, incorporate static analysis into your [regular workflow](https://pvs-studio.com/en/blog/posts/1218/)—and [PVS\-Studio](https://pvs-studio.com/en/pvs-studio/download/) can help you do just that\. You can [request pricing](https://pvs-studio.com/en/order/), a [trial version](https://pvs-studio.com/en/pvs-studio/try-free/), or learn [about free licensing terms](https://pvs-studio.com/en/blog/posts/1286/) in the relevant sections\. Look ahead to your project's future and catch errors as early as possible :\)