C++ two objects holding pointers to different parts of the same array

Question

I try to create two classes, Volume and Slice, and create a method in Volume so it can return part of its data as a Slice object. I want the two objects to share the memory to change any of them will change the same data. Currently, I have:

#include <complex>
using namespace std;


class Slice{
public:
    Slice(unsigned long Nx,unsigned long Ny){   //Contructor
        nx = Nx;
        ny = Ny;
        data = new complex<double>[Nx*Ny];
    }

    Slice(unsigned long Nx,unsigned long Ny,complex<double>* inputDataPtr){ //Contructor
        nx = Nx;
        ny = Ny;
        data = inputDataPtr;
    }

    Slice(const Slice&  inputObj){ // Copy contructor
        nx = inputObj.nx;
        ny = inputObj.ny;
        data = inputObj.data;
    }

    ~Slice(){ //destructor
        delete data;
    }
private:
    // DATA:
    unsigned long     nx;
    unsigned long     ny;
    complex<double>*  data;
};

class Volume{
public:
    Volume(unsigned long Nx,unsigned long Ny,unsigned long Nz){ //Contructor
        nx = Nx;
        ny = Ny;
        nz = Nz;
        data = new complex<double>[Nx*Ny*Nz];
    }

    ~Volume(){ //destructor
        delete data;
    }

    const Slice& get_slice(unsigned long zindex){
            return Slice(nx,ny, &(data[zindex*nx*ny]));
    }
private:
    // DATA:
    unsigned long     nx;
    unsigned long     ny;
    unsigned long     nz;
    complex<double>*  data;
};



int main(){
    unsigned long Nx = 1;
    unsigned long Ny = 2;
    unsigned long Nz = 3;

    Volume testVolume(Nx,Ny,Nz);
    /* initialise data in testVolume */
    Slice slice = testVolume.get_slice(1);

    return 0;
}

I also need to be able to create individual slices that are not related to the volume, so slice class has to allocate its own pointer as well.

When I run this, it says*** free(): invalid pointer: 0x0000000000ff1180 ***

I think the problem is when I call the get_slice method of the testVolume, the code will destroy the temporary slice object in the return, so it tries to free a non-existing pointer and cause problems.

How do I avoid this? After a bit of search, I probably need a smart pointer shared_ptr? How do I incorporate it?

Thank you.

Answer 1

For starters this function

const Slice& get_slice(unsigned long zindex){
        return Slice(nx,ny, &(data[zindex*nx*ny]));
}

has undefined behavior because it returns a reference to a local object of the type Slice that will be deleted after exiting the function.

Secondly this constructor

Slice(unsigned long Nx,unsigned long Ny,complex<double>* inputDataPtr){ //Contructor
    nx = Nx;
    ny = Ny;
    data = inputDataPtr;
}

does not create dynamically the data pointed to by the data member data. So calling the destructor

~Slice(){ //destructor
    delete data;
}

of an object created such a way again can result in undefined behavior.

A similar problem exists with the copy constructor

Slice(const Slice&  inputObj){ // Copy contructor
    nx = inputObj.nx;
    ny = inputObj.ny;
    data = inputObj.data;
}

The same memory can be tried to delete two times by the destructor.

Answer 2

#include <iostream>
#include <memory>
#include <complex>

class Slice{
public:
    Slice(unsigned long Nx, unsigned long Ny) :
        nx(Nx),
        ny(Ny),
        data(std::make_shared<std::complex<double>[]>(Nx*Ny))
    {}

    Slice(unsigned long Nx, unsigned long Ny, std::shared_ptr<std::complex<double>[]> data) :
        nx(Nx),
        ny(Ny),
        data(data)
    {}

private:
    unsigned long     nx;
    unsigned long     ny;
    std::shared_ptr<std::complex<double>[]> data;
};

class Volume{
public:
    Volume(unsigned long Nx, unsigned long Ny, unsigned long Nz) :
        nx(Nx),
        ny(Ny),
        nz(Nz),
        data(std::make_shared<std::complex<double>[]>(Nx * Ny * Nz))
    {
    }

    Slice get_slice(unsigned long zindex)
    {
        // Use std::shared_ptr aliasing constructor
        return Slice(nx, ny,
                     std::shared_ptr<std::complex<double>[]>(data, &data[zindex * nx * ny]));
    }
private:
    // DATA:
    unsigned long     nx;
    unsigned long     ny;
    unsigned long     nz;
    std::shared_ptr<std::complex<double>[]> data;
};



int main(){
    unsigned long Nx = 1;
    unsigned long Ny = 2;
    unsigned long Nz = 3;

    Volume testVolume(Nx,Ny,Nz);
    /* initialise data in testVolume */
    Slice slice = testVolume.get_slice(1);
}

Demo

Answer 3

You problem originates here:

   Slice(unsigned long Nx,unsigned long Ny,complex<double>* inputDataPtr){ //Contructor
        nx = Nx;
        ny = Ny;
        // this is a shallow copy
        data = inputDataPtr;
    }

Now when you delete it here you are actually deleting the one in Volume

 ~Slice(){ //destructor
        delete data;
    }

Once the destuctor of Volume is called you try to delete it again:

  ~Volume(){ //destructor
        delete data;
    }

Which results in the error. To correct this just don't delete it in the Slice destructor.

You should also change this function:

const Slice& get_slice(unsigned long zindex){
            return Slice(nx,ny, &(data[zindex*nx*ny]));
    }

Currently you are returning a reference to a local object which will be destroyed the moment you return. To fix this you should consider either using a raw or a shared_ptr.