c++ - Is there a way to stop a memory leak with a queue and a custom struct with a character array? - Stack Overflow

I am creating a custom object Message, and then appending it to a queue. This will call the copy constructor; however, the addresses after a memmove() don't seem to match like I would expect them to.

The code is as follows

#include <iostream>
#include <queue>

struct Message
{
  char* data = nullptr;
  size_t size = 0;

  Message() = default;

  ~Message()
  {
    delete[] data;
  }

  Message(const Message& other)
  {
      this->data = new char[other.size];
      this->size = other.size;

      memmove(this->data,
              other.data,
              this->size);
  }
};

std::queue<Message> message_queue;

int main()
{
    Message message;

    message.size = 50;
    message.data = new char[message.size];

    std::cout << &message.data << std::endl;

    message_queue.push(std::move(message));

    Message other_message = message_queue.front();
    message_queue.pop();

    std::cout << &other_message.data << std::endl;

    return 0;
}

I am wondering why pushing this item onto the queue and grabbing it's memory address is different. Should the addresses not be the same? And is this a memory leak if I don't define the ~Message() to delete[] data;

edit: This most certainly is a memory leak if i don't delete on destructor. But why is the memory not in the same location?

edit 2:

I now have the class as follows:

#include <iostream>
#include <queue>
#include <cstring>
#include <utility>

struct Message
{
    char* data = nullptr;
    size_t size = 0;

    Message(size_t in_size)
    {
        size = in_size;
        data = new char[size];
    }

    ~Message()
    {
        delete[] data;
    }

    Message(const Message& other)
    {
        std::cout << "Message(const Message& other)\n";

        this->size = other.size;
        this->data = new char[this->size];

        memmove(this->data,
                other.data,
                this->size);
    }

    Message& operator=(const Message& other)
    {
        std::cout << "Message& operator=(const Message& other)\n";

        if (this == &other)
        {
            return *this;
        }

        Message temp(other);

        std::swap(size, temp.size);
        std::swap(data, temp.data);

        return *this;
    }

    Message(Message&& other) noexcept
        : data(std::exchange(other.data, nullptr))
    {
        std::cout << "Message(Message&& other) noexcept\n";
    }

    Message& operator=(Message&& other) noexcept
    {
        std::cout << "Message& operator=(Message&& other) noexcept\n";

        Message temp(std::move(other));

        std::swap(data, temp.data);
        std::swap(size, temp.size);

        return *this;
    }
};

std::queue<Message> message_queue;

int main()
{
    Message message(50);

    message_queue.push(message);

    Message other_message = message_queue.front();
    message_queue.pop();

    std::cout << (void*) message.data << "\n" << (void*) other_message.data << "\n";

    return 0;
}

And my output still is showing that data is pointing to two different locations.

Message(const Message& other)
Message(const Message& other)
0x1b0c2a53400
0x1b0c2a52f40

I believe I am still doing something wrong I am just unsure

I am creating a custom object Message, and then appending it to a queue. This will call the copy constructor; however, the addresses after a memmove() don't seem to match like I would expect them to.

The code is as follows

#include <iostream>
#include <queue>

struct Message
{
  char* data = nullptr;
  size_t size = 0;

  Message() = default;

  ~Message()
  {
    delete[] data;
  }

  Message(const Message& other)
  {
      this->data = new char[other.size];
      this->size = other.size;

      memmove(this->data,
              other.data,
              this->size);
  }
};

std::queue<Message> message_queue;

int main()
{
    Message message;

    message.size = 50;
    message.data = new char[message.size];

    std::cout << &message.data << std::endl;

    message_queue.push(std::move(message));

    Message other_message = message_queue.front();
    message_queue.pop();

    std::cout << &other_message.data << std::endl;

    return 0;
}

I am wondering why pushing this item onto the queue and grabbing it's memory address is different. Should the addresses not be the same? And is this a memory leak if I don't define the ~Message() to delete[] data;

edit: This most certainly is a memory leak if i don't delete on destructor. But why is the memory not in the same location?

edit 2:

I now have the class as follows:

#include <iostream>
#include <queue>
#include <cstring>
#include <utility>

struct Message
{
    char* data = nullptr;
    size_t size = 0;

    Message(size_t in_size)
    {
        size = in_size;
        data = new char[size];
    }

    ~Message()
    {
        delete[] data;
    }

    Message(const Message& other)
    {
        std::cout << "Message(const Message& other)\n";

        this->size = other.size;
        this->data = new char[this->size];

        memmove(this->data,
                other.data,
                this->size);
    }

    Message& operator=(const Message& other)
    {
        std::cout << "Message& operator=(const Message& other)\n";

        if (this == &other)
        {
            return *this;
        }

        Message temp(other);

        std::swap(size, temp.size);
        std::swap(data, temp.data);

        return *this;
    }

    Message(Message&& other) noexcept
        : data(std::exchange(other.data, nullptr))
    {
        std::cout << "Message(Message&& other) noexcept\n";
    }

    Message& operator=(Message&& other) noexcept
    {
        std::cout << "Message& operator=(Message&& other) noexcept\n";

        Message temp(std::move(other));

        std::swap(data, temp.data);
        std::swap(size, temp.size);

        return *this;
    }
};

std::queue<Message> message_queue;

int main()
{
    Message message(50);

    message_queue.push(message);

    Message other_message = message_queue.front();
    message_queue.pop();

    std::cout << (void*) message.data << "\n" << (void*) other_message.data << "\n";

    return 0;
}

And my output still is showing that data is pointing to two different locations.

Message(const Message& other)
Message(const Message& other)
0x1b0c2a53400
0x1b0c2a52f40

I believe I am still doing something wrong I am just unsure

• c++
• memory-management
• queue

Share Improve this question Follow edited Jan 30 at 21:20 Zuiax asked Jan 30 at 20:41 ZuiaxZuiax 6977 bronze badges 20

• You copy construct it twice (so yes, you have three leaks). – Ted Lyngmo Commented Jan 30 at 20:46
• Side note: Looks like you're missing an include. Probably included by one of the other headers on your system, but as you can see here you can't count on that. – user4581301 Commented Jan 30 at 20:50
• 3 Some good reading on Three/Five/zero. – user4581301 Commented Jan 30 at 20:51
• 3 A good way to avoid memory leaks it to avoid calling new/delete yourself. E.g. use std::vector<char> or std::string. Having a member of either of these types will ensure the right things will happen when you move or copy your struct (with raw pointers you have to do a lot of managment yourself, like writing move and copy constructors) – Pepijn Kramer Commented Jan 30 at 20:55
• 2 @Zuiax Something else to be aware of - your code is logging the addresses of the Message::data member itself, not the addresses where data is pointing. message and other_message are separate objects, so the location of their data members is different. Do this instead: std::cout << (void*) message.data << std::endl; ... std::cout << (void*) other_message.data << std::endl; (operator<< is specialized for char* pointers to print out null-terminated C strings). This will give you a better idea of whether or not you end up with different objects pointing at the same char[] memory. – Remy Lebeau Commented Jan 30 at 21:10

 |  Show 15 more comments

1 Answer 1

Sorted by: Reset to default 8

message_queue.push(std::move(message));

This uses the copy constructor you've defined (since you didn't provide a move constructor). Right there, you will allocate new memory and copy the data into that. So, other.data and this->data will point at different addresses.

Message other_message = message_queue.front();

This uses the copy constructor a second time, again allocating new memory and copying the data into that. The new object's data will point at a third address.

You now have three different memory allocations, all being leaked if you don't delete[] the memory in ~Message().

Is there a way to stop a memory leak with a queue and a custom struct with a character array?

Yes, use a std::string or std::vector<char>. Or, if you must manage the memory yourself, then read up on the Rule of 3/5/0 and implement these:

• copy and move constructors
• copy and move assignment operators
• a destructor