John W.
John W.

Reputation: 153

C++ Deep Copy Object

I am trying to deep copy objects back and forth. When I run the gdb, I get the following error after one iteration of the loop.

Program received signal SIGSEGV, Segmentation fault.
0x0804ab96 in DGCPM::DGCPM (this=0x844b760, cur=0x1) at DGCPM.C:27
27    memcpy(vRCells, cur->vRCells,sizeof(float)*nThetaCells);

I suspect the problem has to do with creating the "new class," but I'm not sure. Any suggestions?

(Note: The "_initialize" code calls a FORTRAN subroutine that sets the values in the program.)

Here is the run.C main file:

#include "../include/DGCPM.h"

#define particle_num 5

class DGCPM **mallocModels(int n);

int main(int argc, char *argv[]){

  class DGCPM **m;
  class DGCPM **cur;

  m=mallocModels(particle_num);//update

 for(int t = 0; t < 48; t++){

      //Update m, and then...
      cur = m;
      m = (DGCPM**)malloc(sizeof(class DGCPM *)*particle_num);      
      for(int i=0;i<particle_num;i++){
randomidx = ((double)rand() / ((double)RAND_MAX + 1));
    currentidx = find(cumPw,randomidx,particle_num);
        m[i] = new class DGCPM(cur[currentidx]);
    }
      for(int i=0;i<particle_num;i++){
    delete cur[i];
    }
    free(cur);

  }

   return 0;
}


/*============================================================================
   mallocModels - allocate the ensemble of models
  ============================================================================*/
class DGCPM **mallocModels(int n){
  class DGCPM **m;

  m=(class DGCPM **)amjSafeMalloc(sizeof(class DGCPM *)*n,
              (char *)"mallocModels:m");

  for(int i=0;i<n;i++)
    m[i]=new class DGCPM();

  return m;
}


/*============================================================================
  Find - Return a particle index that has a high probability of having a high weight.
  ============================================================================*/

int find(float *cumPw, double randomidx, int nM){
    /*Wrong implementation*/
        int index = 0;
        flag = 0;
    while(flag == 0){
        if(cumPw[i] >= randomidx){
        flag = 1;
        i++;
    }
        else{
        index ++;
        }
        }
    return index; //Sometimes, index was going to number of models, or number of models + 1, which are out of bounds.
/*Correct implementation*/
    int index = 0;
    for(int i = 0; i < nM-1; i++){
    if(cumPw[i] >= randomidx){
        index = i;
    break;
    }
    }   
    if(index >= nM){
    index = nM-1;
    printf("Error: random index exceeds bounds");
    }
    return index;
}

Here is the DGCPM.h header file:

class DGCPM{

public:
  DGCPM(); /* Initialized with defaults setup */
  DGCPM(class DGCPM *cur); //Copy constructor
  DGCPM(int nThetaCells, int nPhiCells, float thetaMin, float thetaMax); 

  ~DGCPM(); /* Free memory */

private:
  int internal; /* 1=memory allocated internally and should be deallocated when ~DGCPM is called, 2=memory is internal except for mGridN which is external */
  int nThetaCells,nRCells,nPhiCells;
  float thetaMin,thetaMax;
  float rMin,rMax;
  float delR,delPhi;
  float deltMax;
  float *vRCells; /* [nThetaCells] */
  float *vThetaCells; /* [nThetaCells] */
  float *vPhiCells; /* [nPhiCells] */

  float **mGridB; /* [nPhiCells][nThetaCells] */
  float **mGridBi; /* [nPhiCells][nThetaCells] */
  float **mGridPot; /* [nPhiCells][nThetaCells] */
  float **mGridEr; /* [nPhiCells][nThetaCells] */
  float **mGridEp; /* [nPhiCells][nThetaCells] */
  float **mGridVr; /* [nPhiCells][nThetaCells] */
  float **mGridVp; /* [nPhiCells][nThetaCells] */
  float **mGridN; /* [nPhiCells][nThetaCells] */
  float **mGridHalf; /* [nPhiCells][nThetaCells] Particles / weber (workspace for upwind and superbee) */
  float **mGridDen; /* [nPhiCells][nThetaCells] */
  float **mGridVol; /* [nPhiCells][nThetaCells] */
  float **mGridX; /* [nPhiCells][nThetaCells] */
  float **mGridY; /* [nPhiCells][nThetaCells] */
  float **mGridOc; /* [nPhiCells][nThetaCells] */
  float **std; /* [nPhiCells][nThetaCells] */
  float parI[2]; 
  float delTMax;
  float Re;

  void initialize(int nThetaCells, int nPhiCells, float thetaMin, 
          float thetaMax);

};

And finally the DGCPM.C object wrapper:

/******************************************************************************
 * DGCPM.C - This implements the DGCPM plasmasphere model class               *
 ******************************************************************************/
#define TWO_PI 6.2831853071795864769252866
#include "../include/DGCPM.h"
# include <cstdlib>
# include <cmath>

/*============================================================================
  DGCPM::DGCPM()

  Initialize with default setup
  ============================================================================*/
DGCPM::DGCPM(){

  internal=1;

  initialize(200,200,14.963217,60.0);/*(180,200,14.963217,60.0);*/
}

//Copy Constructor
DGCPM::DGCPM(class DGCPM *cur){

  internal=1;

  initialize(200,200,14.963217,60.0);/*(180,200,14.963217,60.0);*/

  memcpy(vRCells, cur->vRCells,sizeof(float)*nThetaCells);
  memcpy(vPhiCells, cur->vPhiCells,sizeof(float)*nPhiCells);
  memcpy(vThetaCells, cur->vThetaCells,sizeof(float)*nThetaCells);
  memcpy(mGridB[0], cur->mGridB[0],sizeof(float)*nThetaCells*nPhiCells);
  memcpy(mGridBi[0], cur->mGridBi[0],sizeof(float)*nThetaCells*nPhiCells);
  memcpy(mGridPot[0], cur->mGridPot[0],sizeof(float)*nThetaCells*nPhiCells);
  memcpy(mGridEr[0], cur->mGridEr[0],sizeof(float)*nThetaCells*nPhiCells);
  memcpy(mGridEp[0], cur->mGridEp[0],sizeof(float)*nThetaCells*nPhiCells);
  memcpy(mGridVr[0], cur->mGridVr[0],sizeof(float)*nThetaCells*nPhiCells);
  memcpy(mGridVp[0], cur->mGridVp[0],sizeof(float)*nThetaCells*nPhiCells);
  memcpy(mGridN[0], cur->mGridN[0],sizeof(float)*nThetaCells*nPhiCells);
  memcpy(mGridHalf[0], cur->mGridHalf[0],sizeof(float)*nThetaCells*nPhiCells);
  memcpy(mGridDen[0], cur->mGridDen[0],sizeof(float)*nThetaCells*nPhiCells);
  memcpy(mGridVol[0], cur->mGridVol[0],sizeof(float)*nThetaCells*nPhiCells);
  memcpy(mGridOc[0], cur->mGridOc[0],sizeof(float)*nThetaCells*nPhiCells);
  memcpy(mGridX[0], cur->mGridX[0],sizeof(float)*nThetaCells*nPhiCells); 
  memcpy(mGridY[0], cur->mGridY[0],sizeof(float)*nThetaCells*nPhiCells);
  memcpy(std[0], cur->std[0],sizeof(float)*nThetaCells*nPhiCells);

} 


/*============================================================================
  DGCPM::~DGCPM()

  Free allocated memory
  ============================================================================*/
DGCPM::~DGCPM(){
  if(internal>=1){
    amjFree1dFloat(vRCells);
    amjFree1dFloat(vThetaCells);
    amjFree1dFloat(vPhiCells);
    amjFree2dFloat(mGridB);
    amjFree2dFloat(mGridBi);
    amjFree2dFloat(mGridEr);
    amjFree2dFloat(mGridEp);
    amjFree2dFloat(mGridVr);
    amjFree2dFloat(mGridVp);
    if(internal==1) amjFree2dFloat(mGridN);
    amjFree2dFloat(mGridHalf);
    amjFree2dFloat(mGridDen);
    amjFree2dFloat(mGridVol);
    amjFree2dFloat(mGridX);
    amjFree2dFloat(mGridY);
    amjFree2dFloat(mGridOc);
    amjFree2dFloat(std);
  }
}


/******************************************************************************
 ******************************************************************************
 **                         Private functions                                **
 ******************************************************************************
 ******************************************************************************/

/*============================================================================
  DGCPM::initialize(int nThetaCells, int nPhiCells, float thetaMin, 
          float thetaMax);

  This is the initialization function used when all memory should be
  allocated internally.
  ============================================================================*/
void DGCPM::initialize(int nThetaCells, int nPhiCells, float thetaMin, 
               float thetaMax){

  initialize(nThetaCells,nPhiCells,thetaMin,thetaMax,
         amjMalloc1dFloat(nThetaCells,(char *)"DGCPM::DGCPM:vRCells"),
         amjMalloc1dFloat(nThetaCells,(char *)"DGCPM::DGCPM:vThetaCells"),
         amjMalloc1dFloat(nPhiCells,(char *)"DGCPM::DGCPM:vPhiCells"),
         amjMalloc2dFloat(nPhiCells,nThetaCells,
                  (char *)"DGCPM::DGCPM:mGridB"),
         amjMalloc2dFloat(nPhiCells,nThetaCells,
                  (char *)"DGCPM::DGCPM:mGridBi"),
         amjMalloc2dFloat(nPhiCells,nThetaCells,
                  (char *)"DGCPM::DGCPM:mGridPot"),
         amjMalloc2dFloat(nPhiCells,nThetaCells,
                  (char *)"DGCPM::DGCPM:mGridEr"),
         amjMalloc2dFloat(nPhiCells,nThetaCells,
                  (char *)"DGCPM::DGCPM:mGridEp"),
         amjMalloc2dFloat(nPhiCells,nThetaCells,
                  (char *)"DGCPM::DGCPM:mGridVr"),
         amjMalloc2dFloat(nPhiCells,nThetaCells,
                  (char *)"DGCPM::DGCPM:mGridVp"),
         amjMalloc2dFloat(nPhiCells,nThetaCells,
                  (char *)"DGCPM::DGCPM:mGridN"),
         amjMalloc2dFloat(nPhiCells,nThetaCells,
                  (char *)"DGCPM::DGCPM:mGridHalf"),
         amjMalloc2dFloat(nPhiCells,nThetaCells,
                  (char *)"DGCPM::DGCPM:mGridDen"),
         amjMalloc2dFloat(nPhiCells,nThetaCells,
                  (char *)"DGCPM::DGCPM:mGridVol"),
         amjMalloc2dFloat(nPhiCells,nThetaCells,
                  (char *)"DGCPM::DGCPM:mGridX"),
         amjMalloc2dFloat(nPhiCells,nThetaCells,
                  (char *)"DGCPM::DGCPM:mGridY"),
         amjMalloc2dFloat(nPhiCells,nThetaCells,
                  (char *)"DGCPM::DGCPM:mGridOc"),
         //Added by J.Wise

         amjMalloc2dFloat(nPhiCells,nThetaCells,
                  (char *)"DGCPM::DGCPM:std"));
}
/*============================================================================
  DGCPM::initialize(int nThetaCells, int nPhiCells, float thetaMin, 
          float thetaMax);

  This is the initialization function used when mGridN is passed from
  the outside but all other memory is allocated internally.
  ============================================================================*/
void DGCPM::initialize(int nThetaCells, int nPhiCells, float thetaMin, 
               float thetaMax, float **mGridN){

  initialize(nThetaCells,nPhiCells,thetaMin,thetaMax,
         amjMalloc1dFloat(nThetaCells,(char *)"DGCPM::DGCPM:vRCells"),
         amjMalloc1dFloat(nThetaCells,(char *)"DGCPM::DGCPM:vThetaCells"),
         amjMalloc1dFloat(nPhiCells,(char *)"DGCPM::DGCPM:vPhiCells"),
         amjMalloc2dFloat(nPhiCells,nThetaCells,
                  (char *)"DGCPM::DGCPM:mGridB"),
         amjMalloc2dFloat(nPhiCells,nThetaCells,
                  (char *)"DGCPM::DGCPM:mGridBi"),
         amjMalloc2dFloat(nPhiCells,nThetaCells,
                  (char *)"DGCPM::DGCPM:mGridPot"),
         amjMalloc2dFloat(nPhiCells,nThetaCells,
                  (char *)"DGCPM::DGCPM:mGridEr"),
         amjMalloc2dFloat(nPhiCells,nThetaCells,
                  (char *)"DGCPM::DGCPM:mGridEp"),
         amjMalloc2dFloat(nPhiCells,nThetaCells,
                  (char *)"DGCPM::DGCPM:mGridVr"),
         amjMalloc2dFloat(nPhiCells,nThetaCells,
                  (char *)"DGCPM::DGCPM:mGridVp"),
         mGridN,
         amjMalloc2dFloat(nPhiCells,nThetaCells,
                  (char *)"DGCPM::DGCPM:mGridHalf"),
         amjMalloc2dFloat(nPhiCells,nThetaCells,
                  (char *)"DGCPM::DGCPM:mGridDen"),
         amjMalloc2dFloat(nPhiCells,nThetaCells,
                  (char *)"DGCPM::DGCPM:mGridVol"),
         amjMalloc2dFloat(nPhiCells,nThetaCells,
                  (char *)"DGCPM::DGCPM:mGridX"),
         amjMalloc2dFloat(nPhiCells,nThetaCells,
                  (char *)"DGCPM::DGCPM:mGridY"),
         amjMalloc2dFloat(nPhiCells,nThetaCells,
                  (char *)"DGCPM::DGCPM:mGridOc"),

         amjMalloc2dFloat(nPhiCells,nThetaCells,
                  (char *)"DGCPM::DGCPM:std"));

}

    /*
      initialize() - this initialization function uses pre-allocated
      memory areas passed in from the outside. This function is used both
      when DGCPM allocates memory itself and when it receives
      pre-allocated memory from the outside in order to eliminate
      duplication of code with the associated risk of errors. 
      ============================================================================*/
    void DGCPM::initialize(int nThetaCells, int nPhiCells, float thetaMin, 
                   float thetaMax, float *vRCells, float *vThetaCells,
                   float *vPhiCells, float **mGridB, float **mGridBi, 
                   float **mGridPot, float **mGridEr, float **mGridEp,
                   float **mGridVr, float **mGridVp, float **mGridN, 
                   float **mGridHalf, float **mGridDen, float **mGridVol,
                   float **mGridX, float **mGridY, float **mGridOc, float **std){

      DGCPM::nThetaCells=nThetaCells;
      DGCPM::nPhiCells=nPhiCells;
      DGCPM::thetaMin=thetaMin;
      DGCPM::thetaMax=thetaMax;
      DGCPM::vRCells=vRCells;
      DGCPM::vThetaCells=vThetaCells;
      DGCPM::vPhiCells=vPhiCells;
      DGCPM::mGridB=mGridB;
      DGCPM::mGridBi=mGridBi;
      DGCPM::mGridPot=mGridPot;
      DGCPM::mGridEr=mGridEr;
      DGCPM::mGridEp=mGridEp;
      DGCPM::mGridVr=mGridVr;
      DGCPM::mGridVp=mGridVp;
      DGCPM::mGridN=mGridN;
      DGCPM::mGridHalf=mGridHalf;
      DGCPM::mGridDen=mGridDen;
      DGCPM::mGridVol=mGridVol;
      DGCPM::mGridX=mGridX;
      DGCPM::mGridY=mGridY;
      DGCPM::mGridOc=mGridOc;
      DGCPM::std=std;
      Re=6.378e6;



      initialize_(&nThetaCells,&nRCells,&nPhiCells,&thetaMin,&thetaMax,&rMin,&rMax,
              &delR,&delPhi,vRCells,vThetaCells,vPhiCells,mGridB[0],mGridBi[0],
              mGridN[0],mGridDen[0],mGridVol[0],mGridX[0],mGridY[0],mGridOc[0],std[0]);
    }

Here's a sample custom memory function, which takes care of initialization and allocation:

void *amjSafeMalloc(int n, char *message){

  void *d;

  d=malloc(n);

  if(d==NULL){
    fprintf(stderr,"amjSafeMalloc error: Could not allocate %d bytes "
        "for %s. Exiting.\n",n,message);
    exit(1);
  }

  return d;
} 

float *amjMalloc1dFloat(int a, char *message){

  float *d;

  sprintf(msg,"%s:amjMalloc1DFloat:d",message);
  d=(float *)amjSafeMalloc(sizeof(float)*a,msg);

  return d;
}

float **amjMalloc2dFloat(int a, int b, char *message){

  float **d;
  int i;

  sprintf(msg,"%s:amjMalloc2DFloat:d",message);
  d=(float **)amjSafeMalloc(sizeof(float *)*a,msg);
  sprintf(msg,"%s:amjMalloc2DFloat:d[0]",message);
  d[0]=(float *)amjSafeMalloc(sizeof(float)*a*b,msg);

  for(i=1;i<a;i++) d[i]=d[i-1]+b;

  return d;
}

Upvotes: 0

Views: 1022

Answers (4)

John W.
John W.

Reputation: 153

This going to sound so stupid (elementary programming error): my index "i" was going beyond (number of models - 1), so I was getting a segmentation fault from accessing memory that didn't exist.

Upvotes: 0

PaulMcKenzie
PaulMcKenzie

Reputation: 35440

The issue is more than likely you're assuming that float** has data that is one contiguous chunk of memory. If so, here is one way of accomplishing this. First, I show the wrong way (but used often):

float** createFloat2D(int nRows, int nCols)
{
    float** p1 = new float*[nRows];
    for (int i = 0; i < nCols; ++i )
       p1[i] =  new float[nCols];
    return p1;
}

void destroyFloat2D(float**f, int nRows, int nCols)
{
   for (int i = 0; i < nCols; ++i )
       delete [] f[i];
   delete [] f;
}

Looks simple, and works for most purposes, but will fail if the assumption is made that the data is in a contiguous chunk of memory.

The other way to create a 2D array is to make the data contiguous.

float** createFloat2D(int nRows, int nCols)
{
    float** p1 = new float*[nRows];  // allocate row pointers
    float* p2 = new float[nRows * nCols];  // allocate data in one chunk
    for (int i = 0; i < nCols; ++i, p2 += nCols )
       p1[i] = p2;  // point the row pointers into the pool of memory
    return p1;
}

void destroyFloat2D(float**f)
{
   delete [] f[0];
   delete [] f;
}

Note above that the data is created in one contiguous "pool". Now, using yourArray[0] actually points to the beginning of this memory. Also note that destruction is done without having to know the number of rows or columns, since f[0] points to the pool of memory.

So now, code like this should work

float** mGridB = createFloat2D(nThetaCells, nPhiCells);
//...
memcpy(mGridB[0], cur->mGridB[0], sizeof(float)*nThetaCells*nPhiCells);

The code above now works correctly, if we use the second method of creating the 2d array.

I would still stick with the vector for 1-d float arrays, as you have the pointer to the data (see my earlier comment). For the code above, I would wrap it in a class that handles creation and destruction easily.

The last thing is the copy constructor. A copy constructor in C++ has the following possible signatures:

DGCPM(const DGCPM&);
DGCPM(DGCPM&);
DGCPM(volatile DBCPM&);

I may have missed one, but the signature should be one of those above, more than likely, the first one (you can also have additional arguments after the reference argument, but they all must have default values).

Note that a DBCPM* is not a valid argument for a copy constructor as your code stated -- remember that a copy constructor is not only for use, but also the compiler will use it to make copies. So to signal the compiler that "yes, this function is used to make copies", your function must match one of the signatures above.

In addition, you need an assignment operator, in other words, the class needs to implement the "rule of 3".

Upvotes: 0

From your comment /* [nPhiCells][nThetaCells] */ in your class definition, I take it that you intent the float** to be 2D arrays. However, if you can use them like 2D arrays, they are actually arrays of pointers to arrays. That is a huge difference: it means, you have to copy nPhiCells individual arrays of nThetaCells elements and you have to setup the pointer array itself. Now, when you do

memcpy(mGridHalf[0], cur->mGridHalf[0],sizeof(float)*nThetaCells*nPhiCells);

in your copy constructor, you assume that there is no pointer array, and that all line arrays are sequential in memory. Either this copy exceeds the bounds of the pointer array (segfaulting), or accessing you array via mGridHalf[i][j] simply does the wrong thing, reinterpreting float data as pointers (and segfaulting).


Unfortunately, C++ is a horrible language for interacting with fortran multidimensional arrays because it has no notion of variable sized arrays. So the following is C code, not C++ code. In C, you can tackle the issue like this:

float (*mGridHalf)[nThetaCells] = malloc(nPhiCells*sizeof(*mGridHalf));

will correctly allocate and type a 2D array (i. e. an array of arrays) that can be accessed with

mGridHalf[phi][theta] = 7.3;

Since all elements are consecutive in memory, the entire thing can correctly be copied with

memcpy(mGridHalf, cur->mGridHalf, nPhiCells*sizeof(*mGridHalf));

and freed with

free(mGridHalf);

Technically, mGridHalf is now a pointer to an array, the pointer arithmetic that is invoked by the array access effectively does the same computation as if you had written:

float* foo = malloc(nPhiCells*nThetaCells*sizeof(*foo));
foo[phi*nThetaCells + theta] = 7.3;

However, using the correct pointer type float (*)[nThetaCells] allows you to avoid doing the index computation yourself.

Upvotes: 0

Neil Kirk
Neil Kirk

Reputation: 21763

class DGCPM
{
public:
    DGCPM(int nThetaCells, int nPhiCells)
    : nThetaCells(nThetaCells)
    , nPhiCells(nPhiCells)
    , mGridB(nThetaCells, vector<float>(nPhiCells)) // first Y then X
    {
    }

private:
    int nThetaCells, nPhiCells;
    vector<vector<float>> mGridB;
};

Deep copies for free. Deletes memory for free.

By free I mean you don't have to write the code..

Upvotes: 1

Related Questions