0

I'm trying to understand the concept behind pointers. I have a function in which i set an pointer to an object as an attribute of a struct:

void util::setobject(string s)
{
   Document* d;
   d->Parse(s.c_str());
   message.payload = d;
{

Outside this function i can call the message object and access the document. Now I call this function for the second time so I overwrite the payload and set a new pointer to a new document. As far as I know, the old document object still exists but nothing points to it correct? Is this object then removed automatically? Thanks!

Edit

Sorry I totally messed up this code example. So I have two options here:

Option 1: Stay with pointers

void util::setobject(string s)
    {
       Document* d = new Document();
       d->Parse(s.c_str());
       message.payload = d;
    {

To avoid a memory leak i would need to delete the pointed object before assigning new value, correct?

void util::setobject(string s)
{
   Document* d = new Document();
   d->Parse(s.c_str());
   delete message.payload; // correct?
   message.payload = d;
{

Second option: Avoid using "new"

void util::setobject(string s)
{
  Document d;
  d.Parse(s.c_str());
  message.payload = d; // object is copied to struct attribute
{

S.th. like that wouldn't work because d goes out of scope:

void util::setobject(string s)
{
  Document d;
  d.Parse(s.c_str());
  message.payload = &d; // does not make sense correct?
{

This might be s.th. off-topic, but the second version would require the struct to look like that:

struct MESSSAGE
{
  string topic;
  bool done = true;
  Document payload;
}

Document comes from <rapidjson/document.h>. Doing it without a pointer results in multiple errors:

error: use of deleted function 'Message::Message(const Message&)'

'rapidjson::GenericDocument<Encoding, Allocator, StackAllocator>::GenericDocument(const rapidjson::GenericDocument<Encoding, Allocator, StackAllocator>&) [with Encoding=rapidjson::UTF8<>; Allocator = rapidjson::MemoryPoolAllocator<>; StackAllocator=rapidjson::CrtAllocator]' is private within this context

Does anyone also know how to interpret these errors? I think that the copy constructor of Document is private, therefore I need to use pointer here, correct?

f_3464gh
  • 122
  • 1
  • 9
  • 4
    This code is invalid, `d` is used uninitialized. – Yksisarvinen Mar 02 '21 at 10:40
  • Document comes from rapdijson an can be used like this – f_3464gh Mar 02 '21 at 10:42
  • 1
    @m_3464gh You don't have an instance of `Document` that could be used for anything. You only have an uninitalized pointer that contains an indeterminate value, i.e. points to a "random" location. – churill Mar 02 '21 at 10:43
  • Where the document comes from is the most important aspect of the issue. You should [edit] your question to include complete, compilable code which demonstrates the situation you're asking about. – Sneftel Mar 02 '21 at 10:44
  • 1
    It may appear to work, if by chance that function doesn't use any data members. But since this is not a `static` function, it's Undefined Behaviour to call it without valid object. – Yksisarvinen Mar 02 '21 at 10:50

3 Answers3

3

Who deletes pointer (not created with new) in C++?

As long as the program is well defined, no one deletes pointer-not-created-with-new because only pointer-created-with-allocating-new may be deleted.


Document* d;
d->Parse(s.c_str());

The behaviour of this example is undefined because you indirect through an uninitialised pointer and attempt to call a member function through it.

message.payload = d;

Here, you copy an indeterminate value. Behaviour is undefined because of this as well.

... an can be used like this

You are mistaken. No pointer can be used like this.

so I overwrite the payload and set a new pointer to a new document.

In the example, you don't set pointer to a "new document". You never created a document in the example.

As far as I know, the old document object still exists

If message.payload used to point to a document object, then that object still exists. You haven't demonstrated such case in the example.

but nothing points to it correct?

Depends. There can be many pointers to an object, so overwriting one pointer doesn't necessarily mean that there are no pointers to the object any more. But indeed, this is a possible case - that wasn't demonstrated in the example.

Is this object then removed automatically?

Depends on the storage class of the object. automatic objects are destroyed automatically (hint is in the name) when they go out of scope. Static objects are destroyed automatically when the program ends. Thread local objects are destroyed automatically when the thread exits. Temporary objects are destroyed automatically at the end of full expression (sometimes extended).

Dynamic objects are not destroyed automatically. They are destroyed only when explicitly destroyed or deleted. If last pointer to dynamic object is overwritten, that object is said to have been leaked. Well, dynamic trivial objects can be destroyed automatically by creating another object in its place, but dynamic memory is not deallocated automatically.


Regarding the edited question;

To avoid a memory leak i would need to delete the pointed object before assigning new value, correct?

Depends on what the old pointer points to. If it points to a dynamic object created with allocating new and there are no other pointers responsible for the deletion, then yes. Otherwise, no.

Does anyone also know how to interpret these errors?

Document is not (publicly) copyable, and you attempted to copy Message which contains such non-copyable document.

eerorika
  • 181,943
  • 10
  • 144
  • 256
2

There is no Document in your code, hence there is nothing you could delete.

What this function does

void util::setobject(string s)
{
   Document* d;
   d->Parse(s.c_str());
   message.payload = d;
}

is the following:

It declares d as a pointer to a Document and leaves that pointer uninitialized. The second line then dereferences that uninitialized pointer and causes undefined behavior.

I suggest you to read Why should C++ programmers minimize use of 'new'?, because your function should actually look like this:

void util::setobject(string s)
{
   Document d;
   d.Parse(s.c_str());
   message.payload = d;
}

If you cannot / do not want to change payload to be a Document (not a pointer), then you need to create a Document:

  Document* d = new Document;

However, you should not use raw pointers here. Look at std::shared_ptr or std::unique_ptr. Raw pointers should not be owning memory. To "own" something means to be responsible to delete it. Smart pointers do that for you, while raw owning pointers are a recipe for desaster and should be avoided (don't misunderstand: raw pointers are fine, the problem is owning raw pointers).

463035818_is_not_a_number
  • 64,173
  • 8
  • 58
  • 126
  • You are of course completely right, i totally messed up the initial code snippet, sry for that! I've edited my code according to your suggestions. You mind taking another look? – f_3464gh Mar 02 '21 at 11:12
  • 1
    @m_3464gh you should not edit your questin to ask for something else after you received answers. The answers refer to your original question not to your new one. You didn't ask the question you wanted to ask, thats a pity, but to not waste the effort that went into the existing answers, the best course of action would be to roll back your edit and open a new question – 463035818_is_not_a_number Mar 02 '21 at 11:13
1

To answer the title question: No-one.

Only pointers that point to objects allocated by new should be deleted and only pointers that point to objects allocated by new[] should be delete[]d.

Pointers to objects that have static1, thread local2 or automatic3 storage duration must not be deleted, the language specifies how and when those objects are destroyed.

  1. "Global" objects
  2. "Per-thread global" objects
  3. "On-the-Stack" objects
Caleth
  • 35,377
  • 2
  • 31
  • 53