Open james-ctc opened 4 months ago
Would it be possible to just use a unique_ptr
for the v2g_context
struct, so that default ctor/dtors are used?
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).
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?
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.
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
).
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:
EVerest Domain
ISO15118
Affected EVerest Module
EvseV2G
To Reproduce
Present in the 2024.3.0 release
Anything else?
No response