# Третья часть исследования Nau Engine

В финальной части нашей трилогии, посвящённой Nau Engine, мы уделим внимание ошибкам, возникающим при разработке классов\. Приведённые в статье примеры наглядно демонстрируют, как даже небольшие недоработки могут обернуться серьёзными проблемами в работе приложения\.

![1244_NauEngineThree_ru/image1.png](https://import.viva64.com/docx/blog/1244_NauEngineThree_ru/image1.png)

Как уже сказано в начале, это третья статья про проверку Nau Engine\. В [первой части](https://pvs-studio.ru/ru/blog/posts/cpp/1221/) мы рассмотрели функционал Nau Engine с акцентом на три группы ошибок: проблемы управления памятью, избыточное копирование кода и логические недочёты\. Во [второй части](https://pvs-studio.ru/ru/blog/posts/cpp/1226/) обсуждались ключевые аспекты оптимизации и повышения производительности\. 

Теперь предлагаю сфокусироваться на ошибках, обнаруженных статическим анализатором PVS\-Studio в процессе разработки классов\.

**Фрагмент N1**

```cpp
template <std::derived_from<Attribute> K, typename T>
struct AttributeField
{
  using Key = K;
  using Value = T;

  T value;

  AttributeField(T&& inValue) :
    value(std::move(value))
  {
  }

  AttributeField(TypeTag<Key>, T&& inValue) :
    value(std::move(inValue))
  {
  }
};
```

Предупреждение PVS\-Studio: [V546](https://pvs-studio.ru/ru/docs/warnings/v546/) Member of a class is initialized with itself: 'value\(std::move\(value\)\)'\. [attribute\.h 118](https://github.com/NauEngine/NauEnginePublic/blob/020cbfec7bd4cff7ff4303e0f31d75eebed584be/engine/core/kernel/include/nau/meta/attribute.h#L118)

В этом коде допущена ошибка, связанная с тем, что в первом конструкторе поле `AttributeField::value` инициализируется самим собой\. В результате переменная используется до своей фактической инициализации, что приводит к неопределённому поведению\. Такой код может привести к неожиданным ошибкам, затруднить отладку и оставить объект в некорректном состоянии\.

Исправленный вариант инициализации должен использовать переданный аргумент `inValue`, а не сам член класса:

```cpp
AttributeField(T&& inValue) :
  value(std::move(inValue))
{
}
```

**Фрагмент N2**

```cpp
class NAU_ANIMATION_EXPORT AnimationInstance : public virtual IRefCounted
{
  ....
private:
  friend class KeyFrameAnimationPlayer;

  AnimationState m_animationState;

  nau::Ptr<Animation> m_animation;
  AnimationAssetRef m_animationAsset;
  ....
  eastl::string m_name;
};

AnimationInstance::AnimationInstance(const eastl::string& name, 
                                     AnimationAssetRef&& assetRef)
  : m_name(name)
{
  m_animationAsset = std::move(assetRef);
}
```

Предупреждение PVS\-Studio: [V730](https://pvs-studio.ru/ru/docs/warnings/v730/) Not all members of a class are initialized inside the constructor\. Consider inspecting: m\_animationState\. [animation\_instance\.cpp 28](https://github.com/NauEngine/NauEnginePublic/blob/020cbfec7bd4cff7ff4303e0f31d75eebed584be/engine/core/modules/animation/src/playback/animation_instance.cpp#L28)

В этом фрагменте рассматривается конструктор класса `AnimationInstance`, который принимает два параметра: имя объекта и rvalue ссылку на `assetRef`\. Обратите внимание, что в списке инициализации и теле конструктора явно задаются лишь значения для `m_name` и `m_animationAsset`, в то время как поле `m_animationState` остаётся без инициализации\. Такая ситуация может привести к неопределённому поведению, поскольку неинициализированные члены класса потенциально содержат мусорные значения из\-за [default\-инициализации](https://en.cppreference.com/w/cpp/language/default_initialization)\. Вероятно, автор кода забыл добавить инициализацию для всех членов класса\. Рекомендуется обеспечить явную инициализацию каждого поля: либо через список инициализации конструктора, либо посредством значений по умолчанию при объявлении, чтобы гарантировать корректное состояние объекта при его создании\.

**Фрагмент N3**

```cpp
class NAU_KERNEL_EXPORT LzmaLoadCB : public IGenLoad
{
public:
  ....
  ~LzmaLoadCB()
  {
    close();
  }
  ....
};

void LzmaLoadCB::close()
{
  if (isStarted && !isFinished && !inBufLeft && rdBufPos >= rdBufAvail)
    ceaseReading();
  ....
}
```

Предупреждение PVS\-Studio: [V1053](https://pvs-studio.ru/ru/docs/warnings/v1053/) Calling the 'ceaseReading' virtual function indirectly in the destructor may lead to unexpected result at runtime\. Check lines: 'dag\_lzmaIo\.h:27', 'lzmaDecIo\.cpp:48', 'dag\_genIo\.h:249'\. [dag\_lzmaIo\.h 27](https://github.com/NauEngine/NauEnginePublic/blob/020cbfec7bd4cff7ff4303e0f31d75eebed584be/engine/core/kernel/include/nau/dag_ioSys/dag_lzmaIo.h#L27)

В этом примере класс `LzmaLoadCB` реализует деструктор, в котором сначала вызывается функция `close`, а затем при выполнении определённого условия происходит вызов функции [`ceaseReading`](https://github.com/NauEngine/NauEnginePublic/blob/020cbfec7bd4cff7ff4303e0f31d75eebed584be/engine/core/kernel/include/nau/dag_ioSys/dag_genIo.h#L249), объявленной как виртуальная в базовом классе `IGenLoad`\. Вызов виртуальных функций в контексте конструирования или разрушения объекта может быть опасным: на этом моменте у объекта либо ещё не инициализированы производные части \(конструирование\), либо уже уничтожены его наиболее производные части \(разрушение\)\. Это означает, что, если у виртуальной функции `ceaseReading` есть переопределение в наиболее производном классе, на момент работы конструктора или деструктора всегда будет вызвана виртуальная функция из текущего или базового класса\. Это может привести к игнорированию важного функционала, реализованного в наиболее производном классе\. Лучше избегать подобных вызовов в конструкторах и деструкторах, чтобы не нарушить корректность работы программы\.

И вот ещё случаи:

* V1053 Calling the 'Type' virtual function indirectly in the constructor may lead to unexpected result at runtime\. Check lines: 318, 252, 245\. [d3dx12\_state\_object\.h 318](https://github.com/NauEngine/NauEnginePublic/blob/020cbfec7bd4cff7ff4303e0f31d75eebed584be/engine/core/modules/render/include/nau/directx/d3dx12_state_object.h#L318)

**Фрагмент N4**

```cpp
FsPath& FsPath::operator=(FsPath&& other)
{
    m_path = std::move(other.m_path);
    other.m_path.clear();
    return *this;
}
```

Предупреждение PVS\-Studio: [V794](https://pvs-studio.ru/ru/docs/warnings/v794/) The assignment operator should be protected from the case of 'this \=\= &other'\. [fs\_path\.cpp 36](https://github.com/NauEngine/NauEnginePublic/blob/020cbfec7bd4cff7ff4303e0f31d75eebed584be/engine/core/kernel/src/io/fs_path.cpp#L36)

В представленном фрагменте реализован оператор перемещения присваивания класса `FsPath`, в котором происходит перенос данных из объекта `other` в текущий экземпляр\. Следует отметить, что отсутствует проверка на самоприсваивание \(проверка вида `(this == &other)`\), что может привести к нежелательным последствиям\.

Если объект пытаются присвоить самому себе, то операция `m_path = std::move(other.m_path);` перемещает содержимое `other.m_path` в `m_path`, а последующий вызов `other.m_path.clear();` очищает данные\. В результате `m_path` оказывается в неожиданном состоянии, и остаётся лишь пожелать удачной отладки :\)

Чтобы устранить этот риск, рекомендую добавить в начало оператора следующую проверку:

```cpp
if (this == std::addressof(other))
{
    return *this;
}
```

Использование `std::addressof` вместо оператора `&` гарантирует корректное сравнение адресов даже при перегрузке оператора `&` в классе\.

**Фрагмент N5**

```cpp
// cocos2d::Node
class CC_DLL Node : public Ref
{
  ....  
  virtual void reorderChild(Node* child, int localZOrder);
  ....
};


// nau::ui::Node
class NAU_UI_EXPORT Node : virtual protected cocos2d::Node
{
  ....
  virtual void reorderChild(Node* child, int zOrder);
  ....
};
```

Предупреждение PVS\-Studio: [V762](https://pvs-studio.ru/ru/docs/warnings/v762/) It is possible a virtual function was overridden incorrectly\. See first argument of function 'reorderChild' in derived class 'Node' and base class 'Node'\. [node\.h 145](https://github.com/NauEngine/NauEnginePublic/blob/020cbfec7bd4cff7ff4303e0f31d75eebed584be/engine/core/modules/ui/include/nau/ui/elements/node.h#L145)

В примере определены два класса с именем `Node`:

* `cocos2d::Node` — оригинальный класс;
* `nau::ui::Node` — класс, который наследуется от `cocos2d::Node`\.

Проблема кроется в том, что функция `reorderChild` объявлена в обоих классах с первым параметром типа `Node`\. Однако по правилам поиска неквалифицированных имён, при объявлении функции в базовом классе имя `Node` будет разрешёно как `cocos2d::Node`, а в производном классе — как `nau::ui::Node`\. Из\-за этого различия компилятор не сделает переопределение виртуальной функции из базового класса\. В результате механизм виртуальных функций сработает некорректно, и при вызове метода через указатель на базовый класс выполняется версия из базового класса, что может привести к непредвиденному поведению программы\.

Чтобы избежать подобных ошибок, рекомендуется при переопределении виртуальных функций явно указать тип первого параметра, а также всегда не забывать о применении спецификатора `override`:

```cpp
// nau::ui::Node
class NAU_UI_EXPORT Node : virtual protected cocos2d::Node
{
  ....
  virtual void reorderChild(cocos2d::Node* child, int zOrder) override;
  ....
};
```

Это обеспечивает полное совпадение сигнатуры метода в производном классе с сигнатурой базового метода, и в случае несовпадения компилятор выдаст ошибку\.

И вот ещё случай:

* V762 It is possible a virtual function was overridden incorrectly\. See first argument of function 'removeChild' in derived class 'Sprite' and base class 'Sprite'\. [sprite\.h 87](https://github.com/NauEngine/NauEnginePublic/blob/020cbfec7bd4cff7ff4303e0f31d75eebed584be/engine/core/modules/ui/include/nau/ui/elements/sprite.h#L87)

**Фрагмент N6**

```cpp
namespace std::chrono
{
    inline void PrintTo(milliseconds t, ostream* os)
    {
        *os << t.count() << "ms";
    }

} // namespace std::chrono
```

Предупреждение PVS\-Studio: [V1061](https://pvs-studio.ru/ru/docs/warnings/v1061/) Extending the 'std' namespace may result in undefined behavior\. [stopwatch\.h 26](https://github.com/NauEngine/NauEnginePublic/blob/020cbfec7bd4cff7ff4303e0f31d75eebed584be/engine/core/kernel/tests/test_common/include/nau/test/helpers/stopwatch.h#L26)

В пространство имён `std::chrono` добавили функцию `PrintTo`\. Эта функция – один из способов [научить GoogleTest](https://github.com/google/googletest/blob/main/docs/advanced.md#teaching-googletest-how-to-print-your-values) печатать значения пользовательских типов, если для них не перегружен оператор `<<`\. Согласно документации, нужно определить эту функцию в том же пространстве имён, где определён наш пользовательский тип\. В данной ситуации, этим типом является специализация шаблона класса `std::chrono::duration` для работы с миллисекундами\.

Расширение стандартного пространства имён `std` \(а также некоторых других пространств, например, `posix`\) — опасная практика, которая может привести к неопределённому поведению\. Проблема в том, что стандарт C\+\+ предполагает неизменность своего пространства имён, а реализация библиотек может со временем расширяться, добавляя новые функции и типы\. Если в `std` или его вложенных пространствах объявить пользовательскую функцию или класс, есть вероятность, что в одной из будущих версий стандарта появится аналогичное определение, что приведёт к конфликтам и нестабильной работе программы\.

Современный стандарт C\+\+ разрешает добавлять сущности в `std` из достаточно [скромного списка](https://en.cppreference.com/w/cpp/language/extending_std)\. И, к сожалению, обычные функции в этот список не входят\. Поэтому поведение будет не определено при добавлении функции `PrintTo` для типа `std::chrono::milliseconds`\. Но что же делать, если мы всё же хотим печатать время в миллисекундах в рамках GoogleTest? Есть два способа решения проблемы\.

**Решение N1**\. В C\+\+20 всё уже сделали за нас, оператор `<<` уже [определён](https://en.cppreference.com/w/cpp/chrono/duration/operator_ltlt) для `std::chrono::duration`\. Осталось лишь дождаться, когда в используемой вами стандартной библиотеке полноценно поддержат проползал [P0355](https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0355r7.html) :\)

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

<details>
   <summary>Нет, не найдёт\\\.</summary>

```cpp
namespace internal_stream_operator_without_lexical_name_lookup {

// The presence of an operator<< here will terminate lexical scope lookup
// straight away (even though it cannot be a match because of its argument
// types). Thus, the two operator<< calls in StreamPrinter will find only ADL
// candidates.
struct LookupBlocker {};
void operator<<(LookupBlocker, LookupBlocker);

struct StreamPrinter {
  template <typename T,
            // Don't accept member pointers here. We'd print them via implicit
            // conversion to bool, which isn't useful.
            typename = typename std::enable_if<
                !std::is_member_pointer<T>::value>::type>
  // Only accept types for which we can find a streaming operator via
  // ADL (possibly involving implicit conversions).
  // (Use SFINAE via return type, because it seems GCC < 12 doesn't handle name
  // lookup properly when we do it in the template parameter list.)
  static auto PrintValue(const T& value, ::std::ostream* os)
      -> decltype((void)(*os << value)) {
    // Call streaming operator found by ADL, possibly with implicit conversions
    // of the arguments.
    *os << value;
  }
};

}  // namespace internal_stream_operator_without_lexical_name_lookup
```



Реализация GoogleTest ограничивает неквалифицированный поиск в обрамляющих пространствах имён \(при помощи перегруженного оператора `operator<<(LookupBlocker, LookupBlocker)`\. А ADL ищет функции только в тех пространствах имён, где объявлены типы аргументов\.


</details>


Поэтому до C\+\+20 и реализации проползала P0355 придётся использовать [код похитрее](https://godbolt.org/z/fd8Pvhrhr)\. Мы унаследуемся от `std::chrono::duration` и разместим наследника в пространстве имён `nau::test`\. Там же мы определим перегрузку оператора `<<`, который будет уметь печатать объекты типа `std::chrono::duration` в произвольный поток вывода\. Далее, когда будет необходимо через GoogleTest напечатать длительность, нужно просто сконвертировать `std::chrono::duration` в нашего наследника\.

<details>
   <summary>Возможная имплементация\\\.</summary>

```cpp
#include <iostream>
#include <chrono>

#define HAS_OVERLOADED_OPERATOR_LTLT_FOR_CHRONO 0
#define HAS_CTAD_FEATURE 0

#ifdef __has_include
  #if __has_include(<version>)
    // use feature test macro
    #include <version>

    // check for the 'operator<<' in the standard library
    #if defined(__cpp_lib_chrono) && __cpp_lib_chrono >= 201907L
      #undef HAS_OVERLOADED_OPERATOR_LTLT_FOR_CHRONO
      #define HAS_OVERLOADED_OPERATOR_LTLT_FOR_CHRONO 1
    #endif

    // check for class template argument deduction feature
    #if defined(__cpp_deduction_guides) && __cpp_deduction_guides >= 201703L
      #undef HAS_CTAD_FEATURE
      #define HAS_CTAD_FEATURE 1
    #endif
  #endif
#endif

namespace nau::test
{
#if !HAS_OVERLOADED_OPERATOR_LTLT_FOR_CHRONO
  template <typename Rep, typename Period>
  struct duration : public std::chrono::duration<Rep, Period>
  {
    using base_class = std::chrono::duration<Rep, Period>;
    using base_class::base_class;

    duration(const base_class &base) : base_class { base } {}
  };

  #if HAS_CTAD_FEATURE
  template <typename Rep, typename Period>
  duration(const std::chrono::duration<Rep, Period> &) -> duration<Rep, Period>;
  #endif

  template <typename Rep, typename Period>
  using duration_wrapper = duration<Rep, Period>;

  using milliseconds_wrapper = duration<std::chrono::milliseconds::rep,
                                        std::chrono::milliseconds::period>;
                                        
  using nanoseconds_wrapper = duration<std::chrono::nanoseconds::rep,
                                       std::chrono::nanoseconds::period>;

  template <typename Rep, typename Period>
  std::ostream& operator<<(std::ostream& out,
                           const std::chrono::duration<Rep, Period>& obj)
  {
    using namespace std::literals;
    
    std::string_view postfix = "s"sv;
    if constexpr (Period::type::num == 1)
    {
      // attoseconds, as
           if constexpr (Period::type::den == 1000000000000000000) 
             postfix = "as"sv;
      // femtoseconds, fs
      else if constexpr (Period::type::den == 1000000000000000) 
             postfix = "fs"sv;
      // picoseconds, ps
      else if constexpr (Period::type::den == 1000000000000) postfix = "fs"sv;
      // nanoseconds, ns
      else if constexpr (Period::type::den == 1000000000) postfix = "ns"sv;
      // microseconds, us
      else if constexpr (Period::type::den == 1000000) postfix = "us"sv;
      // milliseconds, ms
      else if constexpr (Period::type::den == 1000) postfix = "ms"sv;
      // centiseconds, cs
      else if constexpr (Period::type::den == 100) postfix = "cs"sv;
      // deciseconds, ds
      else if constexpr (Period::type::den == 10) postfix = "ds"sv;
    }
    else if constexpr (Period::type::den == 1)
    {
      // minutes, min
           if constexpr (Period::type::num == 60) postfix = "min"sv;
      // hours, h
      else if constexpr (Period::type::num == 3600) postfix = "h"sv;
      // days, d
      else if constexpr (Period::type::num == 86400) postfix = "d"sv;
      // decaseconds, das
      else if constexpr (Period::type::num == 10) postfix = "das"sv;
      // hectoseconds, hs
      else if constexpr (Period::type::num == 100) postfix = "hs"sv;
      // kiloseconds, ks
      else if constexpr (Period::type::num == 1000) postfix = "ks"sv;
      // megaseconds, Ms
      else if constexpr (Period::type::num == 1000000) postfix = "Ms"sv;
      // gigaseconds, ns
      else if constexpr (Period::type::num == 1000000000) postfix = "Gs"sv;
      // teraseconds, ps
      else if constexpr (Period::type::num == 1000000000000) postfix = "Ts"sv;
      // petaseconds, fs
      else if constexpr (Period::type::num == 1000000000000000) 
             postfix = "Ps"sv;
      // exaseconds, Es
      else if constexpr (Period::type::num == 1000000000000000000) 
             postfix = "Es"sv;
    }
  
    out << obj.count() << postfix;
    return out;
  }

#else
  template <typename Rep, typename Period>
  using duration_wrapper = std::chrono::duration<Rep, Period>;

  using milliseconds_wrapper = duration<std::chrono::milliseconds::rep,
                                        std::chrono::milliseconds::period>;

  using nanoseconds_wrapper = duration<std::chrono::nanoseconds::rep,
                                       std::chrono::nanoseconds::period>;
#endif
}
```


</details>


**На посошок**

```cpp
/**
    Test:
        Attributes with non-serializable values ​​and empty string keys
        should not be accessible through the attribute container.
 */
```

Предупреждение PVS\-Studio: [V1076](https://pvs-studio.ru/ru/docs/warnings/v1076/) Code contains invisible characters that may alter its logic\. Consider enabling the display of invisible characters in the code editor\. [test\_runtime\_attributes\.cpp 91](https://github.com/NauEngine/NauEnginePublic/blob/020cbfec7bd4cff7ff4303e0f31d75eebed584be/engine/core/kernel/tests/test_runtime/test_runtime_attributes.cpp#L91)

Анализатор обнаружил, что в комментарии к тесту закралась последовательность байтов `E2 80 8B`, которая интерпретируется как [Zero Width Space](https://ru.wikipedia.org/wiki/%D0%9F%D1%80%D0%BE%D0%B1%D0%B5%D0%BB_%D0%BD%D1%83%D0%BB%D0%B5%D0%B2%D0%BE%D0%B9_%D1%88%D0%B8%D1%80%D0%B8%D0%BD%D1%8B) \(ZWSP\)\. Этот символ представляет собой невидимый пробел, который не отображается на экране, но при этом остаётся частью текста\.

Воспользуемся HEX\-редактором, что убедиться в этом:

![1244_NauEngineThree_ru/image2.png](https://import.viva64.com/docx/blog/1244_NauEngineThree_ru/image2.png)

Сразу хочу сказать, что в этом коде никакого [Trojan Source](https://trojansource.codes/) не заложено\. Однако я хотел бы напомнить читателям, что такая опасность по\-прежнему существует\. Если опасные невидимые символы случайно окажутся или в имени переменной, или строковом литерале, или даже в комментарии, то поведение вашей программы может отличаться от того, что вы видите в коде\. При этом компиляторы прекрасно пропускают такой код без ошибок\.

Хочу также отметить, что не всегда включение отображения невидимых символов в вашем редакторе кода полностью исправит положение\. Троян может затесаться где\-то среди сотен файлов, которые пришли вам в качестве Pull Request\. Поэтому, чтобы защититься от подобных проблем, рекомендуется также применять автоматизированные инструменты\. Здесь выбор остаётся за вами: это может быть и обычный `grep`, а может и статический анализатор кода\.

**Заключение**

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

Надеюсь, что наша трилогия о Nau Engine станет полезным ориентиром для разработчиков, поможет избегать типичных ошибок и повышать качество проектов\. Стоит применять полученные знания на практике, постоянно анализировать и совершенствовать свой код, а также делиться опытом с коллегами\. Ведь успешная разработка это не только владение техническими навыками, но и способность учиться на ошибках и стремиться к постоянному развитию\.

Как говорил [Эдсгер Дейкстра](https://ru.wikipedia.org/wiki/%D0%94%D0%B5%D0%B9%D0%BA%D1%81%D1%82%D1%80%D0%B0,_%D0%AD%D0%B4%D1%81%D0%B3%D0%B5%D1%80_%D0%92%D0%B8%D0%B1%D0%B5): _'Тестирование показывает наличие ошибок, а не их отсутствие'\._ Поэтому важно не только исправлять баги, но и изначально писать код так, чтобы их было как можно меньше\.

Благодарю за внимание\!