Vector of pointers problem

Question

Hi,

I am having quite a bit of trouble with trying to push_back an object of my custom class to a vector of pointers with my custom class as the type. Please see the code below along with the error received. I am using Eclipse with the CDT plugin and OpenCV on windows xp.

I have spent so much time trying to find an answer but to no avail! ps I am a student and pointers etc are not my thing!

    std:: vector<RoadLine>* LaneChangeDetector::roadLines(IplImage* img_8uc1, IplImage* img_8uc3, IplImage* img_edge, std::vector <RoadLine>* roadVector){

 CvMemStorage* storage = cvCreateMemStorage(0);
 CvSeq* lines = 0;
 CvMemStorage* roadStorage = cvCreateMemStorage(0);
 CvSeq* roadLines = 0;

 // Probabalistic Hough transform returns line segments from edge detected image
 lines = cvHoughLines2( img_edge, storage, CV_HOUGH_PROBABILISTIC, 1, CV_PI/180, 50, 200, 200 );

 // Sequence roadlines, lines with correct slope are added to this sequence
 roadLines = cvCreateSeq(0, lines->header_size, lines->elem_size, roadStorage);

 // slope
 double m = 0.0;

 // Point of intersection
 CvPoint poi;

 for(int i = 0; i < lines->total; i++ ){
  CvPoint* line = (CvPoint*)cvGetSeqElem(lines,i);
  CvPoint pt1 = line[0];
  CvPoint pt2 = line[1];

  double x1 = double(pt1.x);
  double y1 = double(pt1.y);
  double x2 = double(pt2.x);
  double y2 = double(pt2.y);

  if(pt1.x == pt2.x){
   m = 1.0;
  }
  else{
   m = (double(y2 - y1)/(double(x2 - x1)));
  }

  if( ((m>0.45) && (m<0.75)) || ((m<-0.45) && (m>-0.75)) ){

   // If the slope is between measured parameters add to roadLines sequence for further analysis
   cvSeqPush(roadLines, line);
  }
 }

 // otherRoadLine used for comparison
 CvPoint* otherRoadLine;

 for(int a=0; a<roadLines->total; a++){

  CvPoint* roadLine = (CvPoint*)cvGetSeqElem(roadLines,a);
  CvPoint rl1 = roadLine[0];
  CvPoint rl2 = roadLine[1];
  int lineCount = 0;

  if(a>0){

   // Test the current line against all the previous lines in the sequence.
   // If the current line is far enough away from all other lines then draw it
   for(int b=0; b<a; b++){
    otherRoadLine = (CvPoint*)cvGetSeqElem(roadLines,b);
    if((roadLine->x > ((otherRoadLine->x) + 200)) || (roadLine->x < ((otherRoadLine->x) - 200)) ){
     lineCount++;
    }
   }
   if(lineCount == a){
    cvLine(img_final, roadLine[0], roadLine[1], CV_RGB(0,0,255), 3, CV_AA, 0 );
    RoadLine myLine = RoadLine(roadLine, 1);
    roadVector->push_back(myLine); //ERROR OCCURS HERE
    cvShowImage("Plate Detection", img_final);
    cvWaitKey(0);
   }
  }
  else{
   cvLine(img_final, roadLine[0], roadLine[1], CV_RGB(0,0,255), 3, CV_AA, 0 );
   RoadLine myLine = RoadLine(roadLine, 1);
   roadVector->push_back(myLine //ERROR OCCURS HERE
   cvShowImage("Plate Detection", img_final);
   cvWaitKey(0);
  }
 }

 if(roadVector->size() >= 2){
  int pos = 0;
  RoadLine line1 = roadVector->at(pos);
  RoadLine line2 = roadVector->at(pos + 1);

  CvPoint* A = line1.line;
  CvPoint p1 = A[0];
  CvPoint p2 = A[1];

  int A1 = p1.y - p2.y;
  int B1 = p1.x - p2.x;
  int C1 = (p1.x*p2.y) - (p1.y*p2.x);

  CvPoint* B = line2.line;
  CvPoint p3 = B[0];
  CvPoint p4 = B[1];

  int A2 = p3.y - p4.y;
  int B2 = p3.x - p4.x;
  int C2 = (p3.x*p4.y) - (p3.y*p4.x);

  int det = A2*B1 - A1*B2;

  if(det == 0){
   printf("Lines are parallel");
  }
  else{
   int x = ( C1*(p3.x - p4.x) - (p1.x - p2.x)*C2 )/det;
   int y = ( C1*(p3.y - p4.y) - (p1.y - p2.y)*C2 )/det;

   poi.x = x;
   poi.y = y;

   horizon = poi.x;

   cvCircle(img_final, poi, 10, CV_RGB(255, 0, 0), 2, CV_AA, 0);
  }
 }

 cvShowImage("Plate Detection", img_final);
 cvWaitKey(0);

 return roadVector;
}

The custom class RoadLine can be seen here

    #include <cv.h>
class RoadLine{
private:
CvPoint* line;
int lane;
public:
RoadLine(CvPoint*, int);
};
RoadLine::RoadLine(CvPoint* aLine, int aLane){
line = aLine;
lane = aLane;
}

From debugging i can see that "std::vector <RoadLine>* roadVector" is being intialised correctly.

Here is what Eclipse tells me:

3 std::vector<RoadLine, std::allocator<RoadLine> >::push_back() F:\MinGW\include\c++\3.4.5\bits\stl_vector.h:560 0x0043e3f9

4 void std::_Construct<RoadLine, RoadLine>() F:\MinGW\include\c++\3.4.5\bits\stl_construct.h:81 0x0044015d

And the program jumps to this section of code in stl_construct.h

  template<typename _T1, typename _T2>
inline void
_Construct(_T1* __p, const _T2& __value)
{
  // _GLIBCXX_RESOLVE_LIB_DEFECTS
  // 402. wrong new expression in [some_]allocator::construct
  ::new(static_cast<void*>(__p)) _T1(__value); //DEBUG THROWS ME TO THIS LINE
}

Again any help would be greatly appreciated.

Cheers

Pat

Answer 1

Your RoadLine class lacks a proper copy-ctor. Now, since you have a member that points to a CvPoint object you have create a copy of the pointer every time you push_back. This is probably not desirable.

RoadLine::RoadLine(const RoadLine & o){
     line = new CvPoint[ 2 ]; 
     line[ 0 ] = o.line[ 0 ];
     line[ 1 ] = o.line[ 1 ];
     lane = o.lane;
}

RoadLine& operator=(const RoadLine & o){
     if (this != &o) { //Remember to check for self-assignment.
      line = new CvPoint[ 2 ]; 
      line[ 0 ] = o.line[ 0 ];
      line[ 1 ] = o.line[ 1 ];
      lane = o.lane;
     }
     return *this;
}

Shorten your code: Try to isolate the problem:

int main() {
    CvPoint pa[] = { CvPoint(0, 0), CvPoint(100, 100) };
    RoadLine rl1(pa, 1);

    vector<RoadLine> v;
    v.push_back(rl1);

    return 0;
}

Does this crash?

Answer 2

You do not use vector of pointers, but vector of objects. In that case, your class needs to have a copy constructor, as push_back stores a copy of object.

As a general debugging advice, try to boil down the problem by removing as much code as you can and still see incorrect behaviour. Try to find the simplest example that fails.

Answer 3

These kinds of errors are usually caused by incorrect memory-management. Sadly, you haven't posted the way how do you manage your memory.

If you can get it run on a linux system, you can try running your program under valgrind, which helps to track down incorrect memory accesses/freeing. Unfortunately, valgrind is not available under windows, but there may be substitutes.

Answer 4

Hi all,

i have changed my class definition of RoadLine to:

#include <cv.h>

class RoadLine{

private:
    int lane;
public:
    CvPoint* line;
    RoadLine(CvPoint*, int);
    RoadLine(const RoadLine &);
    ~RoadLine();
    RoadLine& operator=(const RoadLine & o);
};

RoadLine::RoadLine(CvPoint* aLine, int aLane){
    line = aLine;
    lane = aLane;
}

RoadLine::RoadLine(const RoadLine & myRoadLine){
    line = new CvPoint[ 2 ]; // CRASHES HERE
    line[ 0 ] = myRoadLine.line[ 0 ];
    line[ 1 ] = myRoadLine.line[ 1 ];
    //line = new CvPoint(*myRoadLine.line);
    lane = myRoadLine.lane;
}

RoadLine::~RoadLine(){
    delete line;
}

RoadLine& RoadLine::operator=(const RoadLine & o){
     if (this != &o) { //Remember to check for self-assignment.
      line = new CvPoint[ 2 ];
      line[ 0 ] = o.line[ 0 ];
      line[ 1 ] = o.line[ 1 ];
      lane = o.lane;
     }
     return *this;
}

This is the current version of the RoadLine class

This is how I am implementing the class:

else{
        cvLine(img_final, roadLine[0], roadLine[1], CV_RGB(0,0,255), 3, CV_AA, 0 );
     RoadLine myLine(roadLine, 1);
     roadVector->push_back(myLine); // FROM HERE
     cvShowImage("Plate Detection", img_final);
     cvWaitKey(0);
}

When push_back is called it calls the copy constructor but the program crashes where highlighted above

What difference is made by the fact that my vector is defined;

std::vector<RoadLine>* roadVector

and that i have a CvPoint* not CvPoint[]

sorry if these seem very basic questions

Answer 5

Your new RoadLine class will certainly lead to disaster :

RoadLine::RoadLine(CvPoint* aLine, int aLane){
    line = aLine;
    lane = aLane;
}

RoadLine::RoadLine(const RoadLine & myRoadLine){
    line = myRoadLine.line;
    lane = 1;
}

RoadLine::~RoadLine(){
    delete line;
}

code using it :

                        if(lineCount == a){
                                cvLine(img_final, roadLine[0], roadLine[1], CV_RGB(0,0,255), 3, CV_AA, 0 );
                                RoadLine myLine = RoadLine(roadLine, 1);//create object on the Stack
                                roadVector->push_back(myLine); //Push COPY of myLine
                                cvShowImage("Plate Detection", img_final);
                                cvWaitKey(0);
                        }//Stack-based object "myLine" is automatically destroyed here (leaves scope)

the automatic destruction of "myLine" will delete "myLine.line" (in RoadLine's dtor) but "myLine.line" is still referenced in the vector (you just pushed it).

You have to either make a DEEP COPY of line (as others suggested), something like this :

RoadLine::RoadLine(const RoadLine & myRoadLine){
    line = new CvPoint(*myRoadLine.line);//assuming CvPoint can be copy-constructed
    lane = 1;
}

Or use a CvLine object rather than a pointer (or something else, need more context)

EDIT : Dirk Gently's copy-ctorhas a bug, because it leaks memory to the former "line"-member should be :

RoadLine& operator=(const RoadLine & o){
     if (this != &o) { //Remember to check for self-assignment.
      delete []line;//delete[] vs. delete !
      line = 0;//if next line throws at least we won't double-delete line
      line = new CvPoint[ 2 ]; //this might throw ! should catch (or redesign to get rid of new (prefered)
      line[ 0 ] = o.line[ 0 ];
      line[ 1 ] = o.line[ 1 ];
      lane = o.lane;
     }
     return *this;
}
//consistent constructor !
RoadLine::RoadLine(CvPoint* aLine, int aLane)
    :line(new CvPoint[2]),//might throw, but its better to throw in initializer ! (if you just have one pointer it might be ok to do it like this)
    lane(aLane)
{
     line[0] = aLine[0];
     line[1] = aLine[1];
}
RoadLine::~RoadLine(){
    delete[] line;//also use delete[] vs. normal delete here !
}

EDIT 2 : I almost forgot that I had an idea why it crashes ! maybe you try to build a pair with last and last+1 CvPoint (like this obviously false code)?

CvPoint Pnts[2] = {CvPoint(0,0),CvPoint(1,1)};
Roadline Line(&Pnts[1],1);//tries to access Pnts[2] which is one past end !

Answer 6

The trick with C++ is to imagine the "~" key as big and red and that alarm bells will sound whenever you press it, ie. whenever you're thinking of adding a destructor to a class.

If you're adding a destructor then you NEED a copy constructor and assignment operator. No exceptions. Even if you're not going to copy the object you should still declare them in the private section so the compiler will give errors if they're used accidentally.

You should also use a reference counted pointer instead of a raw C-style pointer whenever the lifetime of an object is being controlled (in C++-speak this is "RAII"). If you did this the destructor would vanish from RoadLine, and, magically, so would your problem.

Answer 7

You don't have a vector of pointers.

std::vector<RoadLine>* roadVector

is a pointer to a vector of RoadLine objects. If you want a vector of pointers, you should do:

std::vector<RoadLine*> roadVector

That may help you (since the vector won't be invoking copy constructors any more), but you should still look at sorting those out as others have suggested.