C++ objects within structure allocated via calloc()

[james-ctc]

Describe the bug

C++ objects should be created via new and removed via delete. This ensures that their constructors and destructors are properly called.

The v2g_context structure in EvseV2G is mostly pointers and basic types where use of calloc() to create the structure is fine.
However there are some std::string objects included that are not correctly constructed. When free() is called on the v2g_context pointer the strings are not destructed and hence there could be a memory leak.

One option would be to split the v2g_context structure into parts that can be managed via calloc/free and parts that require new/delete.

Starting points:

• v2g.hpp for the v2g_context definition
• v2g_ctx.cpp and v2g_ctx.hpp for functions creating and releasing v2g_context

EVerest Domain

ISO15118

Affected EVerest Module

EvseV2G

To Reproduce

Present in the 2024.3.0 release

Anything else?

No response

[a-w50]

Would it be possible to just use a unique_ptr for the v2g_context struct, so that default ctor/dtors are used?

[james-ctc]

Most of the v2g_context needs to be zero initialised. A unique_ptr on it's own doesn't address this.

auto v2g_context = std::make_unique<v2g_context>();
std::memset(v2g_context.get(), 0, sizeof(v2g_context));

would have the problem of overwriting what any constructor would have done.

auto v2g_context = {{0, nullptr, ...},{{},{} ...}; 

would be a pain to maintain.

My suggestion would be to wrap the v2g_context in a class so that the C structures can be safely zero initialised and any C++ objects constructed as usual (and kept apart from the C structures/unions).
That would then be constructed via std::make_unique ...
The functions in v2g_ctx.cpp would become methods and called on construction/deletion/
It may even be possible to minimise the code changes needed by overloading ->

As it stands it isn't a big problem since it looks like v2g_context is only constructed once and std::string is the only class used (at the moment).

[a-w50]

std::make_unique<T> does value initialization (https://en.cppreference.com/w/cpp/language/value_initialization), as it is basically just calling new T, which would result in zero-initialization for primitive types and default construction for class types. So it should actually do what is needed, right?

[james-ctc]

no idea - from reading that it implies that int *p; would mean p == nullptr. A quick bit of sample code and it might be nullptr depending on other things.
From your link:

void f()
{
    A a = A(); // not zero-initialized according to the standard
               // but implementations generate code for zero-initialization nonetheless
}

my take on this - it might be fine. I've no idea.

[a-w50]

The example you're referring to uses a class A which has a defined non-default constructor, which leads to no zero-inititialization (which makes sense, because if you define a constructor by yourself, you probably want to setup things specifically).
But for normal POD types like simple data structs, which have no constructors defined, zero-initialization is guaranteed to happen (in this case it would be actually aggregate initialization). Furthermore if the default constructor is non-trivial (which is the case here, because the struct contains std::string, which has a non-trivial default constructor), then also default-initialization happens after zero-initialization (yielding the proper setup of the string objects).
Also note that even now the std::string objects can potentially yield undefined behavior as their constructor is not called when using calloc (this should be done with in place new).