Question

I have an overloaded = operator which is correctly passing some of the values in the particle class, but not other values. I have troubleshooted online, and could not find anything which directly pertained to this problem. My peer who is fairly competent in C++ could not assist, and recommended I post here. Any assistance would be greatly appreciated.

All the values for width, height, wrap, particlen and array were passing perfectly. However, the values for xpos, ypos, xvel, and yvel were passing with incorrect values. For xpos and ypos, every 11th element was passing correctly, but all the other elements were equal to zero; I expect none of these values to be zero. In main, in the operation immediately proceeding gen1 = gen0;, all the dereferenced values for gen0 were correct. I estimate that they were somehow not passed correctly? I will happily post more code/info if required. Again, any insights would be deeply appreciated. Thank you.

Relevant code:

class particle{             
private:
    int* xpos;  
    int* ypos; 
    int* xvel;
    int* yvel;
    int* array;             
    int width;
    int height; 
    int wrap;
    int particlen;
public:
    void operator=(const particle&);
}

Relevant parts of overloaded = operator:

void particle::operator=(const particle &current){
    int a,b,i,j;
    width = current.width;
    height = current.height;
    wrap = current.wrap;
    particlen = current.particlen;
    array = new int[width*height];
    xpos = new int[particlen];
    ypos = new int[particlen];
    xvel = new int[particlen];
    yvel = new int[particlen];

    for(a=0; a<height; a++){
        for(b=0; b< width; b++){
        this->array[a*width + b] = current.array[a*width + b];
        }
    } 

for(i = 0; i < particlen; i++){
    this->xpos[i] = current.xpos[i];
    this->ypos[i] = current.ypos[i];
    this->xvel[i] = current.xvel[i];
    this->yvel[i] = current.yvel[i];
}

Relevant parts of main:

int main(){
    particle gen0,gen1;
    gen1 = gen0;
}

With Sehe's suggestion, I edited my code. However, I am now getting memory allocation problems with the most simple of commands. The << operator, the first operation called in my main does not allow my array variable to be set equal to an integer in a read in file. I definitely know that the file is readable, as the first output line prints. However, I then get a segmentation fault 11 at array[i*width + j] = k;. GDB prints the output: program received signal EXC_BAD_ACCESS, Could not access memory. Reason: KERN_INVALID_ADDRESS at address: 0x0000000000000000 0x000000010001c4f4 in particle::operator<< (this=0x7fff5fbfcb08, file=0x7fff5fbffa90 "100100") at Before typedef 1 vector.cpp:686 686 array[i*width + j] = k;

I have looked up the correct terminology for the vector library online, and thought my syntax was correct. Any ideas?

Update (9:00 EST 2nd Aug)

I am still having trouble with a NULL pointer. When using gdb to debug, I receive the message:

KERN_INVALID_ADDRESS at address: 0x0000000000000000 0x000000010001d807 in particle::operator<< (this=0x7fff5fbfcb08, file=0x7fff5fbffa90 "5040") at Before typedef 1 vector.cpp:688 688 array[(i*width + j)] = k;

I have little experience with NULL pointers; any suggestions as to why this is happening, and how to fix it? Here is the relevant code (the total code is about 900 lines)

class particle{             
private:
    std::vector<int> xpos;  
    std::vector<int> ypos; 
    std::vector<int> xvel; 
    std::vector<int> yvel; 
    std::vector<int> array; 
    int width;      
    int height;     
    int wrap;       
    int particlen; // number of particles read in
public:
    void operator<<(particle);
    void Collision(int, int, particle); 
    void operator>>(char*);
    void operator<<(char*);
};

void particle::operator<<(char* file) // Reads initial input file
{
ifstream in_file;
in_file.open( file);    // open the file
int i,j,k;
}
in_file >> width >> height >> wrap >> particlen;
    for(i=0; i<height; i++){
        for(j=0; j< width; j++){
            in_file >> k;
            array[i*width + j] = k; // This is line 688 which GDB references
        }
    }
}

void particle::operator<<(particle current){
int i,k,j,l;

for(k = 0; k < height*width; k++)
{
    array[k] = 0;
}

k = 0;

for(i=0; i < particlen; i++)
{
    cout << "Current x pos is" << current.xpos[i] << endl;
}

for(i=0; i < particlen; i++)
{

    if(array[ (current.ypos[i]-1)*width + (current.xpos[i] - 1) ] == 0 )
    {
    //cout << "Current X position[" << i << "] is" << current.xpos[i] << endl;
        xpos[i] = current.xpos[i] + (current.xvel[i])*timeinc;
        if(xpos[i] > width) xpos[i] = xpos[i] - width; 
        ypos[i] = current.ypos[i] + (current.yvel[i])*timeinc;
        if(ypos[i] > width) ypos[i] = ypos[i] - height;

    //cout << "Next X position[" << i << "] is" << xpos[i] << endl;
    }

    if(array[ (current.ypos[i]-1)*width + (current.xpos[i] - 1) ] > 1 && current.JC[i] != 1)
    {
        for(l=i+1; l < particlen; l++)
        {
            if(current.xpos[l] == current.xpos[i] && current.ypos[l] == current.ypos[i] && current.JC[l] != 1)
            {
            Collision(i,l,current);
            }
        }
    }
}

for(i=0; i < particlen; i++)
{
    array[ (xpos[i]-1)*width + (ypos[i]-1) ] = 1;
}
Display();}

int main()
{
    char filename[256];
    particle gen0, gen1;
    gen0 << filename;
    gen1 = gen0;
    gen0 << gen1; // Calculates the next state of gen0, based on the values in gen1.
}

I know this is long, but I thought that this is the minimal information that I could provide. I thought that perhaps the problem is with the gen0 << gen1 operator, so I included the method.

Was it helpful?

Solution

Like I hinted in the comment, 99% says you're violating Rule Of Three

Notably, you have the assignment operator, but there's no copy constructor in sight.. This could mean that if copied, the array pointers will end up "illegally" shared. I also assume you have a destructor somewhere that deletes those arrays (in one instance), thus invalidating the arrays that were copied into another instance.

Fix it by avoiding manual memory management in the first place:

#include <vector>

class particle{             
    std::vector<int> xpos;  
    std::vector<int> ypos; 
    std::vector<int> xvel;
    std::vector<int> yvel;
    std::vector<int> array;             
    int width;
    int height; 
    int wrap;
    int particlen; // or use `xpos.size()` e.g.
};

int main()
{
    particle gen0,gen1;
    gen1 = gen0;
}

PS. You might want to consider using v.at(i) instead of v[i] whenever you want bounds-checking. This can be considerable help in debugging issues.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top