# Exploring Nau Engine codebase: pt\.3

In the final part of our trilogy on Nau Engine, we'll focus on errors that occur when implementing classes\. The examples in this article show how even small defects can lead to serious issues in the application operation\.

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

As mentioned at the beginning, this is the third article on checking Nau Engine\. In the [first part](https://pvs-studio.com/en/blog/posts/cpp/1221/), we focused on the engine's functionality, highlighting three key error categories: memory issues, copy\-paste code, and logic errors\. The [second part](https://pvs-studio.com/en/blog/posts/cpp/1226/) discussed important aspects of optimization and performance improvement\. 

Now, let's talk about errors in class implementation that the PVS\-Studio static analyzer has detected\.

**Fragment 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))
  {
  }
};
```

The PVS\-Studio warning: 

[V546](https://pvs-studio.com/en/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)

This code fragment contains an error: the `AttributeField::value` data member is initialized by itself\. As a result, the variable is used before it's actually initialized, which leads to undefined behavior\. Such code can cause unexpected errors, make debugging more difficult, and leave the object in an invalid state\.

To fix the initialization, developers should use the passed `inValue` argument, not the class member:

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

**Fragment 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);
}
```

The PVS\-Studio warning: 

[V730](https://pvs-studio.com/en/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)

This snippet contains the constructor of the `AnimationInstance` class, which takes two parameters: an object name and an rvalue reference to `assetRef`\. Notice that only the values for `m_name` and `m_animationAsset` are explicitly set in the initialization list and constructor body, while the `m_animationState` data member remains uninitialized\. This can lead to undefined behavior because uninitialized class data members may contain garbage values due to the [default\-initialization](https://en.cppreference.com/w/cpp/language/default_initialization)\. The code author might have forgotten to initialize all the class data members\. To ensure the object is in a valid state upon creation, it's recommended to explicitly initialize every class data member—either in the constructor initialization list or by providing default values at declaration\.

**Fragment N3**

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

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

The PVS\-Studio warning: 

[V1053](https://pvs-studio.com/en/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)

In this example, the `LzmaLoadCB` class implements a destructor where the `close` member function is called first, and then the `[ceaseReading](https://github.com/NauEngine/NauEnginePublic/blob/020cbfec7bd4cff7ff4303e0f31d75eebed584be/engine/core/kernel/include/nau/dag_ioSys/dag_genIo.h#L249)` member function, declared as virtual in the `IGenLoad` base class, is called when a certain condition is met\. Calling virtual class functions in the context of object construction or destruction can be dangerous: at this point, the object has either not yet initialized its most derived parts \(construction\) or has already destroyed them \(destruction\)\. This means that if the `ceaseReading` virtual function is overridden in the most derived class, a virtual function from the current or base class will always be called at the moment of construction or destruction\. This may lead to ignoring important functionality implemented in the derived class\. To preserve correct program behavior, it's better to avoid such calls in constructors or destructors\.

Another case:

* 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)

**Fragment N4**

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

The PVS\-Studio warning: 

[V794](https://pvs-studio.com/en/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)

The move assignment operator for the `FsPath` class, which transfers data from the `other` object to the current instance, is implemented in the code fragment\. However, there's no check for self\-assignment \(i\.e\. `(this == &other)`\), which could lead to unintended consequences\.

If an attempt is made to assign the object to itself, the `m_path = std::move(other.m_path);` operation moves the contents of `other.m_path` into `m_path`, and then the subsequent call to `other.m_path.clear();` clears the data\. As a result, `m_path` ends up in an unexpected state, and one can only wish developers happy debugging :\)

To eliminate the risk, we recommend adding the following check at the beginning of the operator:

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

Using `std::addressof` instead of the `operator &` ensures correct address comparison even when the `operator &` is overloaded in the class\.

**Fragment 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);
  ....
};
```

The PVS\-Studio warning:

[V762](https://pvs-studio.com/en/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)

The example contains two classes named `Node`:

* `cocos2d::Node` is the original class;
* `nau::ui::Node` is the class derived from `cocos2d::Node`\.

The issue arises because the `reorderChild` member function is declared in both classes with a parameter of the `Node` type\. However, in the base `Node` class, this parameter is a pointer to the original `cocos2d` class, whereas in the derived class, it's the same class\. This discrepancy may prevent the compiler from recognizing a member function in the derived class as a valid override of the base class member function\. As a result, the virtual function mechanism fails, and when a function is called via a pointer to the base class, a version of the base class is executed, potentially leading to unexpected program behavior\.

To avoid such errors, it's recommended to explicitly specify `namespace` in the function parameter types and always use the `override` specifier when overriding virtual functions:

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

This ensures that the function signature in the derived class matches the base function signature, and if it doesn't, the compiler will show an error\.

Here's another case:

* 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)

**Fragment N6**

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

} // namespace std::chrono
```

The PVS\-Studio warning:

[V1061](https://pvs-studio.com/en/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)

A developer has added the `PrintTo` function to the `std::chrono` namespace\. This function is one of the ways to [teach GoogleTest](https://github.com/google/googletest/blob/main/docs/advanced.md#teaching-googletest-how-to-print-your-values) to print values of custom types if the `operator<<` is not overloaded for them\. According to the documentation, we need to define this function in the same namespace where our custom type is defined\. In this case, this type is a specialization of the `std::chrono::duration` class template for working with milliseconds\.

Extending the standard `std` namespace \(or other reserved namespaces like `posix`\) is a questionable practice that can lead to [undefined behavior](https://pvs-studio.com/en/blog/terms/0066/)\. The issue is that the C\+\+ standard expects its namespace to remain unchanged, while library implementations may evolve over time, introducing new functions and types\. If a function or class is declared within `std` or its nested namespaces, there's a chance that a future version of the standard may introduce an identical definition, leading to conflicts and unstable program behavior\.

The modern C\+\+ standard allows adding entities to `std` from a rather [small list](https://en.cppreference.com/w/cpp/language/extending_std)—and, unfortunately, free functions are not included\. Therefore, adding the `PrintTo` function for the `std::chrono::milliseconds` type will result in undefined behavior\. But what should we do if we still want to print time in milliseconds within GoogleTest? There are two ways to solve the problem\.

**Solution N1**\. C\+\+20 has already done everything for us, the `[operator<<](https://en.cppreference.com/w/cpp/chrono/duration/operator_ltlt)` is defined for `std::chrono::duration`\. All that's left is to wait for the standard library you are using to fully support the [P0355](https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0355r7.html) proposal :\)

**Solution N2**\. Well, since the standard library doesn't have this operator yet, we'll define it ourselves in the global namespace\. And then, the compiler will find the operator we need, right?

<details>
   <summary>No, it won't\\\.</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
```



The GoogleTest implementation restricts unqualified name lookup in enclosing scopes \(this is done by using the overloaded `operator<<(LookupBlocker, LookupBlocker)`\)\. As a result, ADL will only find functions in namespaces where argument types are declared\.


</details>


That's why we have to use [trickier code](https://godbolt.org/z/fd8Pvhrhr) before C\+\+20 and the implementation of P0355\. We will define a new class derived from `std::chrono::duration` in the `nau::test` namespace\. In that same namespace, we will also define an overload of the `operator<<` to print objects of type `std::chrono::duration` to an arbitrary output stream\. Then, when it's necessary to print a duration via GoogleTest, we just need to convert `std::chrono::duration` into our derived class\.

<details>
   <summary>Possible implementation\\\.</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>


**One last thing**

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

The PVS\-Studio warning: 

[V1076](https://pvs-studio.com/en/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)

In the test comment, the analyzer detected a byte sequence `E2 80 8B`, which is interpreted as [Zero Width Space](https://en.wikipedia.org/wiki/Zero-width_space) \(ZWSP\)\. This character is an invisible space that is not displayed on the screen but remains part of the text\.

Let's use a HEX editor to see for ourselves:

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

We would like to say right away that this code does not contain any [Trojan Source](https://trojansource.codes/)\. But it's worth reminding everyone that this danger still exists\. If some dangerous invisible characters accidentally appear in a variable name, or a string literal, or even in a comment, your program may behave differently than expected—without showing any errors during compilation\.

However, enabling the display of invisible characters in your code editor will not always fix the situation\. A trojan may be somewhere among the hundreds of files that have been sent to you as a pull request\. Therefore, to protect yourself from such problems, it's recommended to use automated tools\. Here the choice is yours: it can be an ordinary `grep` or a static code analyzer\.

**Conclusion**

The examples above clearly show how even small errors can escalate into serious issues at runtime\. Flaws like incorrect member initialization, errors in overriding virtual functions, or extending standard namespaces can lead to vulnerabilities\. Detecting and addressing such vulnerabilities in time is key to developing stable and reliable software\.

We hope our trilogy on Nau Engine will provide developers with a useful reference, helping them avoid common mistakes and enhance project quality\. Applying this knowledge in real\-world situations, continuously analyzing and refining code, and sharing insights with colleagues are all essential steps toward professional growth\. After all, successful development isn't just about technical skills—it's about learning from mistakes and striving for continuous improvement\.

[Edsger W\. Dijkstra](https://en.wikipedia.org/wiki/Edsger_W._Dijkstra) once said, "_Program testing can be used to show the presence of bugs, but never to show their absence\."_ That's why it's important not just to fix bugs but to write code in a way that minimizes them from the start\.

Thank you for reading\!