Go Premium for a chance to win a PS4. Enter to Win

x
?
Solved

Invalid pointer hunt

Posted on 2004-09-29
5
Medium Priority
?
411 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
ID: 12178440
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
ID: 12178758
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 2000 total points
ID: 12178856
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
ID: 12187738
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
ID: 12187877
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

How to Use the Help Bell

Need to boost the visibility of your question for solutions? Use the Experts Exchange Help Bell to confirm priority levels and contact subject-matter experts for question attention.  Check out this how-to article for more information.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

When writing generic code, using template meta-programming techniques, it is sometimes useful to know if a type is convertible to another type. A good example of when this might be is if you are writing diagnostic instrumentation for code to generat…
Article by: SunnyDark
This article's goal is to present you with an easy to use XML wrapper for C++ and also present some interesting techniques that you might use with MS C++. The reason I built this class is to ease the pain of using XML files with C++, since there is…
The viewer will learn how to user default arguments when defining functions. This method of defining functions will be contrasted with the non-default-argument of defining functions.
The viewer will learn how to clear a vector as well as how to detect empty vectors in C++.
Suggested Courses

782 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