c++ overloading =operator, RAII

Question

I'm trying to do a simple buffer and follow the RAII idiom

in main.cpp

int main()
{
    buffer buf(5);
    buffer test(10);
    test = buf;


    return 0;
}

in buffer.cpp

#include "buffer.h"
#include <iostream>
size_t buffer::get_size() const
{
    return length;
}
buffer::buffer(size_t length) : start(new int[length]), length(length)
{
    std::cout << length << +" size" << std::endl;
}

buffer::buffer(const buffer& rhs) : start(new int[rhs.get_size()]), length(rhs.get_size())
{
    std::copy(rhs.begin(), rhs.end(), start);
}

buffer& buffer::operator=(const buffer& rhs)
{
    buffer temp = rhs;
    std::swap(*this, temp);

    return *this;

}

int* buffer::begin()
{
    return start;
}

int* buffer::end()
{
    return start + length;
}

const int* buffer::begin() const
{
    return start;
}

const int* buffer::end() const
{
    return start + length;
}
buffer::buffer(buffer&& rhs) noexcept
{
    *this = std::move(rhs);
}
buffer& buffer::operator=(buffer&& rhs) noexcept
{
    if (this != &rhs) { // if this is not the rhs object
        start = rhs.start;
        length = rhs.length;
        rhs.start = nullptr;
        rhs.length = 0;
    }

    return *this;
}
buffer::~buffer()
{
    delete[] start;
}

in buffer.h

#pragma once
class buffer {
    size_t length;
    int* start;

public:
    size_t get_size() const;
    explicit buffer(size_t size);
    buffer(const buffer& rhs);
    buffer& operator=(const buffer& rhs); 
    buffer& operator=(buffer&& rhs) noexcept;
    buffer(buffer&& rhs) noexcept;
    int* begin(); 
    int* end();
    const int* begin() const;
    const int* end() const;
    ~buffer();
};

Now as you notice in main buf is a smaller size than test. My question is what happens to the memory allocated by test on the line above test=buf?

Does it ever get cleaned up? Or do main have to finish before it gets cleaned up.

Does this answer your question? [What is the copy-and-swap idiom?](https://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom) And https://stackoverflow.com/questions/4421706/what-are-the-basic-rules-and-idioms-for-operator-overloading — Richard Critten, Nov 14 '19 at 16:20
Did you actually write this code? If so, you should know what your copy assignment operator does. — NathanOliver, Nov 14 '19 at 16:20
@NathanOliver-ReinstateMonica I did but the copy assignment operator does not seem to remove the memory allocated by test when the copy assignment operator goes out of scope. — kino92, Nov 14 '19 at 16:22
BTW, your copy constructor is invalid, you use `length` before it's initialized: `start(new int[length]), length(rhs.get_size())` (class members are always initialized in order of declaration in class, and `start` is declared before `lnegth`) — Yksisarvinen, Nov 14 '19 at 16:23
@FredLarson ops forgot to include the deconstructor fixed that now — kino92, Nov 14 '19 at 16:24
@Yksisarvinen do you mean in the header file? so the header file should be size_t length; int* start; — kino92, Nov 14 '19 at 16:25
Or initialize `start` using the length from `rhs`: `start(new int[rhs.length])`. It's going to be less error prone than ordering members correctly. — Yksisarvinen, Nov 14 '19 at 16:27
@kino92 -- You need to have a working, non-buggy, copy constructor and destructor before utilizing copy / swap in the assignment operator. Obviously this was not the case. — PaulMcKenzie, Nov 14 '19 at 16:37
You can get rid of the resource leak in the move-assignment operator, by applying a solution I first came to know in [Arthur O'Dwyer's CppCon 2019 talk](https://youtu.be/7Qgd9B1KuMQ?t=2145): Instead of implementing both a copy-assignment and move-assignment, just supply `buffer& buffer::operator=(buffer rhs)` (note the pass-by-value semantics). This allows you to apply the copy-and-swap idiom without checking for self-assignment. And it solves another problem, too (watch the whole video to find out, which problem that is). — IInspectable, Nov 14 '19 at 16:57
@Yksisarvinen I would avoid initializing start *or* length in the copy constructor. Instead, delegate to the non copy: `:buffer(rhs.legnth) { std::copy(rhs.begin(), rhs.end(), start); }` — Martin Bonner supports Monica, Nov 15 '19 at 14:58

score 1 · Answer 1 · answered Nov 14 '19 at 16:32

The original array allocated by test will be swapped to the temp object in the copy assignment, and freed when that object goes out of scope.

But the move assignment has a memory leak, you have to free the old array before acquiring the new one from rhs.

And generally, I would recommend using unique_ptr instead of manual allocation.

c++ overloading =operator, RAII

1 Answers1