destruction of already destructed pointer variable when copying anobject - abort error

[suresh]

Hi,
Kindly consider the code segment below: I have a function object
containing a pointer variable which is passed to min_element
algorithm. My problem is the pointer variable is deleted twice and I
do not know how to fix this issue.

class FO{
public:
set<int>::size_type size(){return s->size()}
bool operator()(int a, int b);
~FO();

private:
set<int> * s;
};

FO::FO(){
s = new set<int>;
}

FO::~FO(){
delete s;
}

bool FO:perator()(int a,int b){
s->insert(a);
return (a<b);
}

int main(){
vector<int> v;
//vector populated
FO fo;
min_element(v.begin(),v.end(),fo);
}

The variable 's' is defined as a pointer bcz min_element algorithm
takes a copy of its function object argument. Now inside the
min_element algorithm, the copy of 'fo' is deleted which results in
freeing of the memory associated with 's'. But in the main, the
original object fo is destructed and then also the same memory is
freed and this gives a abort error.

How to solve this kind of a problem?

thanks
suresh

 

[Garrett Hartshaw]

Hi,

Kindly consider the code segment below: I have a function object
containing a pointer variable which is passed to min_element
algorithm. My problem is the pointer variable is deleted twice and I
do not know how to fix this issue.

class FO{
public:
set<int>::size_type size(){return s->size()}
bool operator()(int a, int b);
~FO();

private:
set<int> * s;
};

FO::FO(){
s = new set<int>;
}

FO::~FO(){
delete s;
}

bool FO:perator()(int a,int b){
s->insert(a);
return (a<b);
}

int main(){
vector<int> v;
//vector populated
FO fo;
min_element(v.begin(),v.end(),fo);
}

The variable 's' is defined as a pointer bcz min_element algorithm
takes a copy of its function object argument. Now inside the
min_element algorithm, the copy of 'fo' is deleted which results in
freeing of the memory associated with 's'. But in the main, the
original object fo is destructed and then also the same memory is
freed and this gives a abort error.

How to solve this kind of a problem?

I'm no expert, but I would say that a smart pointer (e.g.
std::tr1::shared_pointer) would probably be your best choice.

-Garrett

 

[Gil]

Hi,
Kindly consider the code segment below: I have a function object
containing a pointer variable which is passed to min_element
algorithm. My problem is the pointer variable is deleted twice and I
do not know how to fix this issue.

class FO{
public:
set<int>::size_type size(){return s->size()}
bool operator()(int a, int b);
~FO();

private:
set<int> * s;

};

FO::FO(){
s = new set<int>;

}

FO::~FO(){
delete s;

}

bool FO:perator()(int a,int b){
s->insert(a);
return (a<b);

}

int main(){
vector<int> v;
//vector populated
FO fo;
min_element(v.begin(),v.end(),fo);

}

The variable 's' is defined as a pointer bcz min_element algorithm
takes a copy of its function object argument. Now inside the
min_element algorithm, the copy of 'fo' is deleted which results in
freeing of the memory associated with 's'. But in the main, the
original object fo is destructed and then also the same memory is
freed and this gives a abort error.

How to solve this kind of a problem?

thanks
suresh

solution no. 89 to your question:

/*
* func_obj.cpp
*
* Created on: Sep 11, 2010
* Author: Gill
*/

#include <iostream>
#include <iterator>
#include <set>
#include <vector>
#include <algorithm>
#include <tr1/functional>
#include <cstdlib>

struct func_obj : public std::binary_function< int, int, bool > {

typedef std::set< int >::const_iterator set_citerator_t;

public:
bool operator( )( int a, int b ) {
s_.insert( a );
return ( a < b );
}

std::set< int >::size_type size( ) {
return s_.size( );
}

set_citerator_t begin( ) const {
return s_.begin( );
}

set_citerator_t end( ) const {
return s_.end( );
}

private:
std::set< int > s_;

};

int main( ) {
std::vector< int > v;
//vector populated
std::generate_n( back_inserter( v ), 10, std::rand );

func_obj fo;

std::min_element( v.begin( ), v.end( ), std::tr1::ref( fo ) );
std::copy( fo.begin( ),
fo.end( ),
std:stream_iterator< int >( std::cout, " " ) );

std::cout << std::endl;

return 0;
}

 

[Goran]

Hi,
Kindly consider the code segment below: I have a function object
containing a pointer variable which is passed to min_element
algorithm. My problem is the pointer variable is deleted twice and I
do not know how to fix this issue.

class FO{
public:
set<int>::size_type size(){return s->size()}
bool operator()(int a, int b);
~FO();

private:
set<int> * s;

};

FO::FO(){
s = new set<int>;

}

FO::~FO(){
delete s;

}

bool FO:perator()(int a,int b){
s->insert(a);
return (a<b);

}

int main(){
vector<int> v;
//vector populated
FO fo;
min_element(v.begin(),v.end(),fo);

}

The variable 's' is defined as a pointer bcz min_element algorithm
takes a copy of its function object argument. Now inside the
min_element algorithm, the copy of 'fo' is deleted which results in
freeing of the memory associated with 's'. But in the main, the
original object fo is destructed and then also the same memory is
freed and this gives a abort error.

How to solve this kind of a problem?

1. understand "the rule of three" (http://en.wikipedia.org/wiki/
Rule_of_three_%28C%2B%2B_programming%29)
2. class FO : public boost::noncopyable { ...

Goran.

 

[Juha Nieminen]

1. understand "the rule of three" (http://en.wikipedia.org/wiki/
Rule_of_three_%28C%2B%2B_programming%29)

Minor nitpick, but I think it's a bit incorrect to say that
"if a class defines one of the following it should probably explicitly
*define* all three" (emphasis mine).

In many cases it's sufficient to *declare* (rather than define) the
copy constructor and assignment operator (and declare them private), if
your class is such that it requires a user-defined destructor. Often this
is a much easier and less laborious way of adding the safety measure
without having to actually go through the trouble of fully implementing
copying (especially since in many cases there's no clear "best" approach
at copying an object; should it use deep-copying, lazy copying, or just
reference counting, or perhaps something else completely?)

2. class FO : public boost::noncopyable { ...

Is there any other advantage of boost:noncopyable other than that it
saves you from writing two lines of code in the private section of the
class? (And no, "it gives a better error message" is not a good enough
argument for requiring Boost to be installed in the system. Anybody who
is even half-competent at C++ will understand what it means if the copy
constructor or assignment operator of a class is inaccessible because it
has been declared private.)

 

[Kai-Uwe Bux]

Hi,
Kindly consider the code segment below: I have a function object
containing a pointer variable which is passed to min_element
algorithm. My problem is the pointer variable is deleted twice and I
do not know how to fix this issue.

class FO{
public:
set<int>::size_type size(){return s->size()}
bool operator()(int a, int b);
~FO();

private:
set<int> * s;
};

FO::FO(){
s = new set<int>;
}

FO::~FO(){
delete s;
}

bool FO:perator()(int a,int b){
s->insert(a);
return (a<b);
}

int main(){
vector<int> v;
//vector populated
FO fo;
min_element(v.begin(),v.end(),fo);
}

The variable 's' is defined as a pointer bcz min_element algorithm
takes a copy of its function object argument.

I take it, you want to inspect the set after min_element() has run.
Otherwise, the above would not be a rationale.

Now inside the
min_element algorithm, the copy of 'fo' is deleted which results in
freeing of the memory associated with 's'. But in the main, the
original object fo is destructed and then also the same memory is
freed and this gives a abort error.

How to solve this kind of a problem?

Don't have FO take ownership and use pointers only to manage addresses of
objects that already exist. Something like this (off the cuff and untested):

typedef std::set< int > int_set;

class FO {
int_set * where;
public:
FO ( int_set & ref )
: where ( &ref )
{}
bool operator() ( int lhs, int rhs ) const {
where->insert(a);
return ( a < b );
}
}

int main ( void ) {
std::vector< int > v;
...
int_set inspect;
min_element( v.begin(), v.end(), FO( inspect ) );
// now inspect contains some elements
}


Best

Kai-Uwe Bux

 

[Kai-Uwe Bux]

1. understand "the rule of three" (http://en.wikipedia.org/wiki/
Rule_of_three_%28C%2B%2B_programming%29)

That hint points out the problem, ...

2. class FO : public boost::noncopyable { ...

.... but fo is passed by value into min_element(), so would not a copy be
necessarily made?


Best

Kai-Uwe Bux

 

[Öö Tiib]

  Is there any other advantage of boost:noncopyable other than that it
saves you from writing two lines of code in the private section of the
class? (And no, "it gives a better error message" is not a good enough
argument for requiring Boost to be installed in the system. Anybody who
is even half-competent at C++ will understand what it means if the copy
constructor or assignment operator of a class is inaccessible because it
has been declared private.)

Oh that is probably too harsh. Boost is often better than current half-
baked C++0x support gradually becoming available. In fact most experts
have boost (or more likely several revisions of it) installed anyway.
The reason is usually not noncopyable, true. Several libraries in
boost may be restricted by policy (questionable quality), but i have
heard no-one restricting usage of noncopyable. It is ~40 lines of
mostly comments that does not cause any code or data with most
compilers and it certainly documents the design intentions well
enough. Only thing i am not sure is why to use "public" inheritance
from it.

 

[Öö Tiib]

Öö Tiib ha scritto:


You don't have to use public inheritance. You can just write

class Example : boost::noncopyable
{

};

Yes, one of the very few classes for what specifying access adds
nothing. I was questioning about Goran's original "public" (you
snipped):

2. class FO : public boost::noncopyable { ...

"public" is noise, name of class looks like macro and noncopyable does
not fit to OP problem, but Juha's response felt a bit like throwing
baby away instead of dirty water. ;-)

 

[suresh]

I'm no expert, but I would say that a smart pointer (e.g.
std::tr1::shared_pointer) would probably be your best choice.

thanks Garret. Is there any difference between boost::shared_ptr and
std::tr1::shared_pointer? I am hearing about both for the first time.
suresh

 

[suresh]

solution no. 89 to your question:

/*
 * func_obj.cpp
 *
 *  Created on: Sep 11, 2010
 *      Author: Gill
 */

#include <iostream>
#include <iterator>
#include <set>
#include <vector>
#include <algorithm>
#include <tr1/functional>
#include <cstdlib>

struct func_obj : public std::binary_function< int, int, bool > {

  typedef std::set< int >::const_iterator set_citerator_t;

  public:
    bool operator( )( int a, int b ) {
      s_.insert( a );
      return ( a < b );
    }

    std::set< int >::size_type size( ) {
      return s_.size( );
    }

    set_citerator_t begin( ) const {
      return s_.begin( );
    }

    set_citerator_t end( ) const {
      return s_.end( );
    }

  private:
    std::set< int > s_;

};

int main( ) {
  std::vector< int > v;
  //vector populated
  std::generate_n( back_inserter( v ), 10, std::rand );

  func_obj fo;

  std::min_element( v.begin( ), v.end( ), std::tr1::ref( fo ) );
  std::copy( fo.begin( ),
             fo.end( ),
             std:stream_iterator< int >( std::cout, " " ) );

  std::cout << std::endl;

  return 0;



}

thanks for the code. I have not studied std::tr1 so far. I guess it is
time to look at that.
suresh

 

[suresh]

1. understand "the rule of three" (http://en.wikipedia.org/wiki/
Rule_of_three_%28C%2B%2B_programming%29)
2. class FO : public boost::noncopyable { ...

Thanks Goran for pointing out the rule of three which I was not that
clearly aware and I was not carefully following in my coding also. I
guess notcopyable will not be ok for me because I want to access the
min_element modified function object after the execution of
min_element algorithm.
thanks again
suresh

 

[suresh]

I take it, you want to inspect the set after min_element() has run.
Otherwise, the above would not be a rationale.



Don't have FO take ownership and use pointers only to manage addresses of
objects that already exist. Something like this (off the cuff and untested):

typedef std::set< int > int_set;

class FO {
  int_set * where;
public:
  FO ( int_set & ref )
    : where ( &ref )
  {}
  bool operator() ( int lhs, int rhs ) const {
    where->insert(a);
    return ( a < b );
  }

}

int main ( void ) {
  std::vector< int > v;
  ...
  int_set inspect;
  min_element( v.begin(), v.end(), FO( inspect ) );
  // now inspect contains some elements

}

thanks for the code!
suresh

 

[suresh]

I'm no expert, but I would say that a smart pointer (e.g.
std::tr1::shared_pointer) would probably be your best choice.

Garret this means that I can avoid having a destructor if I use the
shared_pointer right?
thanks
suresh

 

[Balog Pal]

I didnt understand. If I do not allocate memory for s in set set<int>
* s, how can I fill the set?

The simplest is that the set is a local object in a function. The function
that calls for_each with your functor.

{
set<int> myset;
for_each( ..., f(&set))
...
}

Or it can be member of another object.

Dynamic allocation is also possible but is needed quite rarely. So is
shared_ptr.

 

[suresh]

solution no. 89 to your question:

/*
 * func_obj.cpp
 *
 *  Created on: Sep 11, 2010
 *      Author: Gill
 */

#include <iostream>
#include <iterator>
#include <set>
#include <vector>
#include <algorithm>
#include <tr1/functional>
#include <cstdlib>

struct func_obj : public std::binary_function< int, int, bool > {

  typedef std::set< int >::const_iterator set_citerator_t;

  public:
    bool operator( )( int a, int b ) {
      s_.insert( a );
      return ( a < b );
    }

    std::set< int >::size_type size( ) {
      return s_.size( );
    }

    set_citerator_t begin( ) const {
      return s_.begin( );
    }

    set_citerator_t end( ) const {
      return s_.end( );
    }

  private:
    std::set< int > s_;

};

int main( ) {
  std::vector< int > v;
  //vector populated
  std::generate_n( back_inserter( v ), 10, std::rand );

  func_obj fo;

  std::min_element( v.begin( ), v.end( ), std::tr1::ref( fo ) );
  std::copy( fo.begin( ),
             fo.end( ),
             std:stream_iterator< int >( std::cout, " " ) );

  std::cout << std::endl;

  return 0;



}

Thanks for this solution. I finally accepted your solution for my
problem. Just one qn. This code will not work if func_obj is not
derived from binary_function. Why is this so?
thanks
suresh

 

[Garrett Hartshaw]

thanks Garret. Is there any difference between boost::shared_ptr and
std::tr1::shared_pointer? I am hearing about both for the first time.
suresh

They are the same thing, just in different namespaces. it started in
boost, then was accepted by the standard committee in namespace
std::tr1, and has now been accepted in namespace std for c++0x.
-Garrett

P.S. my name has two 't's

 

[Garrett Hartshaw]

I'm no expert, but I would say that a smart pointer (e.g.

Garret this means that I can avoid having a destructor if I use the
shared_pointer right?

Yes, because the shared_pointer deletes the memory when it goes out of
scope.
-Garrett

 

[suresh]

P.S. my name has two 't's

Garrett, thanks and sorry for not correctly spelling your name!
suresh

 

[James Kanze]

Garret this means that I can avoid having a destructor if I use the
shared_pointer right?

No. It means that you don't have to write one yourself. You
still have a destructor; it's just that the compiler generates
the correct one automatically.