[Webinar] Streamline your web hosting managementRegister Today

x
  • Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 412
  • Last Modified:

Invalid pointer hunt

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
loveslave
Asked:
loveslave
  • 3
  • 2
1 Solution
 
SteHCommented:
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
 
loveslaveAuthor Commented:
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
 
SteHCommented:
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
 
loveslaveAuthor Commented:
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
 
SteHCommented:
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

2018 Annual Membership Survey

Here at Experts Exchange, we strive to give members the best experience. Help us improve the site by taking this survey today! (Bonus: Be entered to win a great tech prize for participating!)

  • 3
  • 2
Tackle projects and never again get stuck behind a technical roadblock.
Join Now