Should I check and free pointer in VLA class' assign operator (operator=)

Question

So let's say we have a simple variable-length array class like following:

struct VLA {
    double* d=nullptr;
    int dim;
}

What makes me wonder is, within operator=, should I check (and perhaps free/delete if not nullptr) d before malloc/new an new array for it? As assignment is different from copy constructor where it might originally also carried an array.

Like following example:

operator=(VLA &other) {
    double *tmp=new double[dim];
    memcpy(tmp, other.d, sizeof(double)*other.dim);
    delete[]d;
    d=tmp;
    dim=other.dim;
}

Is the delete[]d required?

VLA refers to some specific construct available in C. Also the free and malloc tags are not that relevant here — 463035818_is_not_an_ai, Jul 22 '21 at 15:33
Does this answer your question? https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three. Well, not directly i suppose, but you need more than that `delete` to have a class that is not horribly broken — 463035818_is_not_an_ai, Jul 22 '21 at 15:35
maybe you already have more of the special member functions. You should include them in the question, because they are closely related. — 463035818_is_not_an_ai, Jul 22 '21 at 15:36
When reassigning objects it is your duty to free old memory to prevent a leak. `delete` will safely handle null pointers. You do not need to assert that `new` is not null, it will throw an exception if it cannot allocate. If you want `new` to return null when allocation fails use `new(::std::nothrow) double[]`. — vandench, Jul 22 '21 at 15:37
for example without a proper constructor the `delete` might be called with `d` uninitialized. — 463035818_is_not_an_ai, Jul 22 '21 at 15:37
@Andrew.Wolphoe *should I check (and perhaps free/delete if not nullptr) d before malloc/new an new array for it?* -- If you have a proper copy constructor and destructor, then there is no need to do any checking. `VLA temp(other); std::swap(temp.d, d), std::swap(temp.dim, dim);` -- Those three lines alone do all the work you're doing now. — PaulMcKenzie, Jul 22 '21 at 15:54

score 0 · Answer 1 · answered Jul 22 '21 at 20:27

within operator=, should I check (and perhaps free/delete if not nullptr) d before malloc/new an new array for it?

Is the delete[]d required?

If you are going to allocate a new array with new[], then yes, you need to free the the old array with delete[], otherwise it will be leaked. Whether you do that before or after allocating the new array is up to you, but I would do it after, in case allocating the new array fails.

Note that you can optimize the code you have shown a little, by skipping the new[]/delete[] if the new dim is the same value as the old dim, and by using the copy-swap idiom when you do allocate a new array:

VLA(const VLA &other) {
    d = new double[other.dim];
    memcpy(d, other.d, sizeof(double) * other.dim);
    dim = other.dim;
}

VLA& operator=(const VLA &other) {
    if (this != &other) {
        if (dim == other.dim) {
            memcpy(d, other.d, sizeof(double) * other.dim);
        else {
            VLA temp(other);
            std::swap(d, temp.d);
            std::swap(dim, temp.dim);
        }
    }
    return *this;
}

Though, you really should should be using std::vector instead of new[]/delete[] manually at all. Let std::vector handle the array for you:

#include <vector>

struct VLA {
    std::vector<double> d;
    int dim() const { return d.size(); }

    // compiler-generated copy constructor will "do the right thing"...
    // compiler-generated operator=() will "do the right thing"...
};

Should I check and free pointer in VLA class' assign operator (operator=)

1 Answers1