Is here a memory leak?:
#include <iostream>
#include <string>
class A{
std::string value;
public:
A(){
char *tmp = new char[3];
tmp[0] = 'a';
tmp[1] = 'b';
tmp[2] = 'c';
value = std::string(tmp);
}
std::string const& getValue() const {
return value;
}
};
int main(){
A a;
std::cout << a.getValue() << '\n';
}
since the value is constructed from pointer to newed memory, and thus the std::string is taking the ownership of it, will it also destroy? Or is it my responsibility to take care of that temporary object? And if so, the only way to destroying it is via having it as private variable, otherwise it goes out of scope (since the pointer is with auto storage created in constructor function).
So how is here the buffer managed?
CodePudding user response：
The answer is no. I mean imagine if it did and you did this (ignoring the missing 0)
char *tmp = new char[3];
tmp[0] = 'a';
tmp[1] = 'b';
tmp[2] = 'c';
std::string value1 = std::string(tmp);
std::string value2 = std::string(tmp);
You would be very upset if the second string creation failed
Also std::string(const char*s) cannot tell that the memory for s is on the heap, stack or static. All it can see is a pointer. Even if it wanted to steal the memory it has no way of knowing if its doable
CodePudding user response：
thus the
std::stringis taking the ownership of it
This assumption is incorrect. std::string doesn't take ownership of any pointer, constructor taking char* copies the data into internal storage. So yes, you have to delete[] what you new[]'ed.
Also, if you are passing char* to std::string, it must be null terminated:
A(){
char *tmp = new char[4];
tmp[0] = 'a';
tmp[1] = 'b';
tmp[2] = 'c';
tmp[3] = '\0';
value = std::string(tmp);
}
CodePudding user response：
And thus the std::string is taking the ownership of it, will it also destroy?
If you take ownership of a resource, that means that you will eventually release the resource (unless you transfer it to some other owner, and unless the ownership is shared). If you don't release the resource (or the mentioned exceptions apply), then either you have not taken ownership, or you have a bug.
std::string will only delete a buffer that std::string has created. std::string will not take ownership of a buffer that you have created. Your assumption of "std::string is taking the ownership" is wrong.
Never try to guess whether some function / constructor takes ownership or not. Always rely on documentation to find that out.
Or is it my responsibility to take care of that temporary object?
Yes. You are responsible for freeing the allocations that you create (until you transfer the onwership to something like a smart pointer).
The dynamic allocation that you created leaks, because you don't deallocate it.
And if so, the only way to destroying it is via having it as private variable, otherwise it goes out of scope
You can simply delete it after the std::string has been created and thus your buffer is no longer used.
Important! std::string constructor that accepts char* requires that the pointed string is null terminated. Your string doesn't contain the null terminator character, and thus the call violates the precondition. The behaviour of your program is undefined.
Furthermore, the allocation is entirely unnecessary. You can achieve the intended behaviour and fix both the undefined behaviour and the memory leak by writing the constructor like this:
A(): value("abc") {}
but what if I get the char pointer from another function that malloced it
Primarily, try to avoid calling such functions.
But in case you have no other option, then you must understand that the function is transferring the ownership of the allocation to you (I cannot think of any other reason the function would document that it used std::malloc). You must deallocate it using std::free once you no longer need it. Ideally, you should use something like a smart pointer to do so.
I used uuid_unparse which gives char pointer generated from malloc (see its documentation)
According to the documentation of uuid_unparse, it doesn't do such thing. It doesn't allocate anything. If you want to use it with std::string, you can do this:
uuid_t some_uuid = example_uuid();
constexpr std::size_t uuid_size = 36;
std::string str(uuid_size, '\0');
uuid_unparse(some_uuid, str.data());
CodePudding user response：
The 5th constructor explained in the documentation takes the char* argument. It means it takes the pointer pointing the c-style string, calculates its length and then converts to std::string. The memory of original c-style char* string isn't released, so you have memory leak unless you later delete the char* string somewhere in code.
CodePudding user response：
Most interesting comment:
@MarekR the point was, I tried to replicate the error I got. I used
uuid_unparsewhich gives char pointer generated from malloc (see its documentation), but in class handling uuid I had std::string. And since the std::string does not have ownership, I had to change the implementation of the class to handle char pointers only – milanHrabos
Apparently you are using uuid_unparse with c in wrong way and when problem appeared you asked question which do not reflect actual problem.
I would use this API in following way:
std::string uuid_to_string(uuid_t uu)
{
char buff[37];
uuid_unparse(uu, buff);
return buff; // implicit conversion to std::string is done here.
}
or more fancy way to do it:
std::string uuid_to_string(uuid_t uu)
{
std::string buff(size_t{36}, '\0');
uuid_unparse(uu, buff.data()); // since C 17 data returns `char *`
return buff;
}
About your code:
char *tmp = new char[3];
tmp[0] = 'a';
tmp[1] = 'b';
tmp[2] = 'c';
value = std::string(tmp);
this has 3 issues:
- you are allocating memory which is not freed (string doesn't take over ownership of it) - so memory leak
- your buffer do not contain terminating zero, so when it is passed to
std::stringthis leads to undefined behavior since and of string can't be determined - code is simply over-complicated. After understudying what was its purpose I was able to provide simple solution.