Solved

Invalid pointer hunt

Posted on 2004-09-29
5
367 Views
Last Modified: 2012-05-05
OK, this might be tricky, but I'll post all the code I think is relevant and hope that someone is willing to take the time and see if they can find what's wrong with my code.

I'm implementing a triangle mesh class (which currently uses the boost graph library for internal storage). My problem is that something seems to be wrong with my code for inserting vertices into the mesh. When loading a mesh from file, it seems to work just fine, but when using the same add_vertex function to copy select vertices from one mesh to another, only garbage gets inserted.

I think it must be a problem with scoping in some way, but I can't quite see what goes wrong.

Are these code snippets enough to see what's going on?

////////////////////////////////////////////////

using namespace std;

int main( )
{
  Mesh<3> m;   // Create two three-dimensional triangel meshes.
  Mesh<3> m2;

   m.load( "data/file1.wrl" );
   //m2.load( "data/file2.wrl" );     // If m2 is intialized like this, all is fine
   m2 = ICP_Tools::random_subset( m, 10 ); // If m2 is initialized like this, weird things happen
 
    IO::output << "\n Vertices:\n" ;
    Mesh<3>::Vertex_Iterator vertex( m.vertices().first );
    Mesh<3>::Vertex_Iterator vertex_end( m.vertices().second );
    for (  ; vertex != vertex_end;  ++vertex )
    {
      IO::output << *vertex << "\n";  // The outputs here look fine
    }

    IO::output << "\n Vertices:\n" ;
    vertex = m2.vertices().first;
    vertex_end = m2.vertices().second ;
    for (  ; vertex != vertex_end;  ++vertex )
    {
      IO::output << *vertex << "\n"; // The outputs here are vertices with random numbers, or a seg fault
    }
   
  return 0;
}

////////////////////////////////////////////////

template< unsigned int TDim >
class Mesh
{
  // snip...

  void load( const string file_name )
  {  
    // Open file, some basic setup, start reading values...

    Vertex<3> mesh_point;
    vector< long double > p;

    while (fscanf( in_file, "%Lf %Lf %Lf,", &p[0], &p[1], &p[2] ))
      {
      nof_points++;
      mesh_point.set_position( p );
      this->add_vertex( mesh_point );
      }

    // Do more reading, close file...
  }

  void add_vertex( Vertex<TDim> v )
  {
    Graph_Vertex vv = boost::add_vertex( its_vertices );
    its_vertex_data[vv] = v; // All of this is pass-by-value, so a copy of the vertex v should be copied here, right?
  }

  // snip...
};

////////////////////////////////////////////////

namespace ICP_Tools
{

Mesh<3> random_subset( Mesh<3> const& mesh, unsigned int size )
{
  Mesh<3> the_subset;

  // Use STL random sample algorithm to pick points:
  Vertex<3> temp_holder[size];
  Mesh<3>::Const_Vertex_Iterator first_vertex = mesh.const_vertices().first;
  Mesh<3>::Const_Vertex_Iterator last_vertex  = mesh.const_vertices().second;
  __gnu_cxx::random_sample_n( first_vertex, last_vertex,
                        temp_holder, size );

  for (unsigned int i = 0; i < size; ++i)
    {
      the_subset.add_vertex( temp_holder[i] );   // Here, vertices are copied to the new Mesh.
    }

  return the_subset;
}

} // end namespace ICP_Tools
0
Comment
Question by:loveslave
  • 3
  • 2
5 Comments
 
LVL 13

Expert Comment

by:SteH
Comment Utility
the_subset is a local variable which will go out of scope when returning from the function random_subset. You either need to add a reference to m2 as a parameter to the call or return a mesh allocated on the heap:

Mesh<3>* random_subset( Mesh<3> const& mesh, unsigned int size )
{
  Mesh<3>* the_subset = new Mesh<3>[size];

  // Use STL random sample algorithm to pick points:
  Vertex<3> temp_holder[size];
  Mesh<3>::Const_Vertex_Iterator first_vertex = mesh.const_vertices().first;
  Mesh<3>::Const_Vertex_Iterator last_vertex  = mesh.const_vertices().second;
  __gnu_cxx::random_sample_n( first_vertex, last_vertex,
                     temp_holder, size );

  for (unsigned int i = 0; i < size; ++i)
    {
     the_subset->add_vertex( temp_holder[i] );   // Here, vertices are copied to the new Mesh.
    }

  return the_subset;
}
And don't forget to delete [] the returned value when it is no longer used.
} // end namespace ICP_Tools
0
 
LVL 1

Author Comment

by:loveslave
Comment Utility
Ah, but I thought that the_subset would be copied when returning from the function. Isn't it?

It does work when I supply the_subset as an out argument instead, though.
0
 
LVL 13

Accepted Solution

by:
SteH earned 500 total points
Comment Utility
The return value will be copied. For a built in type it will the value. But in the case of  a complex variable the adress will be copied. And that address is pointing on the stack which will most likely be overwritten. In that case the address is pointing to garbage as you found out.

If you supply the_subset as output parameter it will work. That case is more or less equal to the case that you add a reference to m2: only keep in mind that you either need a pointer to that variable or a reference so that changes to its structure are reflected back to the calling function:

Mesh<3>* random_subset( Mesh<3> const& mesh, Mesh<3>& out_mesh, const unsigned int size )
          The first parameter could be passed as value as well but it more efficient only coipying the address.
          Second parameter needs to be reference
          Third parameter should be const, shouldn't it?
0
 
LVL 1

Author Comment

by:loveslave
Comment Utility
I feel a bit worried now... I've been programming for many years, and this hasn't really come up before. Are you saying that you can never safely return anything but a POD-type? What about std::vector and the like?

Would it not be safe to use the return value from the following function?
Vertex<3> translate( const Vertex<3>& v )
{
  Vertex<3> result;
  result = some_operation_on( v );
  return result;
}
0
 
LVL 13

Expert Comment

by:SteH
Comment Utility
You can return other types, you are right. I think you will need a copy constructor and assignment operator for Mesh<3>. If this is missing a default one is used which copies built-in types and for the rest addresses.
In that case you need to add
Mesh<3> (&Mesh<3>) {...};
Mesh<3>& operator= ... {...};
to your class.


0

Featured Post

What Is Threat Intelligence?

Threat intelligence is often discussed, but rarely understood. Starting with a precise definition, along with clear business goals, is essential.

Join & Write a Comment

Errors will happen. It is a fact of life for the programmer. How and when errors are detected have a great impact on quality and cost of a product. It is better to detect errors at compile time, when possible and practical. Errors that make their wa…
This article shows you how to optimize memory allocations in C++ using placement new. Applicable especially to usecases dealing with creation of large number of objects. A brief on problem: Lets take example problem for simplicity: - I have a G…
The viewer will learn how to use the return statement in functions in C++. The video will also teach the user how to pass data to a function and have the function return data back for further processing.
The viewer will learn additional member functions of the vector class. Specifically, the capacity and swap member functions will be introduced.

744 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

17 Experts available now in Live!

Get 1:1 Help Now