throw from c'tor

Discussion in 'C++' started by Chameleon, Nov 12, 2006.

  1. Chameleon

    Chameleon Guest

    Is there a possibility to create memory leak, the code below if I run
    the line:
    ---------------------------------------------------------
    MyClass cl = new MyClass();
    ---------------------------------------------------------
    ???

    ---------------------------------------------------------
    MyClass::MyClass()
    {
    try {
    int *a = 0;
    int *b = 0;
    a = new int[X];
    b = new int[X];
    } catch (...) {
    if (a) delete[] a;
    if (b) delete[] b;
    throw;
    }
    }
    ---------------------------------------------------------

    I know, it is not so clever to throw things from a c'tor.

    But code above has any problem?


    thanks!


    PS: I worked in C++ for 3 years but after I worked in Java for a 3 years
    too. So the returning is a bit hmmm...
    Chameleon, Nov 12, 2006
    #1
    1. Advertising

  2. Chameleon wrote:
    > Is there a possibility to create memory leak, the code below if I run
    > the line:
    > ---------------------------------------------------------
    > MyClass cl = new MyClass();


    did you mean:

    MyClass * cl = new MyClass();

    ?

    > ---------------------------------------------------------
    > ???
    >
    > ---------------------------------------------------------
    > MyClass::MyClass()
    > {
    > try {
    > int *a = 0;
    > int *b = 0;
    > a = new int[X];
    > b = new int[X];
    > } catch (...) {
    > if (a) delete[] a;
    > if (b) delete[] b;

    delete on a null pointer is OK - no need to do null check.
    > throw;
    > }
    > }
    > ---------------------------------------------------------
    >
    > I know, it is not so clever to throw things from a c'tor.
    >
    > But code above has any problem?


    a better idea is to use a vector...

    MyClass::MyClass()
    {
    vector<int> a(X);
    vector<int> b(X);
    }

    .... much easier to read and much harder to get wrong

    >
    >
    > thanks!
    >
    >
    > PS: I worked in C++ for 3 years but after I worked in Java for a 3 years
    > too. So the returning is a bit hmmm...
    Gianni Mariani, Nov 12, 2006
    #2
    1. Advertising

  3. Chameleon

    peter koch Guest

    Chameleon skrev:
    > Is there a possibility to create memory leak, the code below if I run
    > the line:
    > ---------------------------------------------------------
    > MyClass cl = new MyClass();
    > ---------------------------------------------------------
    > ???


    No.

    >
    > ---------------------------------------------------------
    > MyClass::MyClass()
    > {
    > try {
    > int *a = 0;
    > int *b = 0;
    > a = new int[X];
    > b = new int[X];
    > } catch (...) {
    > if (a) delete[] a;
    > if (b) delete[] b;
    > throw;
    > }
    > }


    As Gianni said, it is much better to use a std::vector. It is more
    flexibe and just as fast as raw pointers. Most important, however is
    that it will "automate" all your work. Copy constructing and assignment
    can all be made to work by using the compilers built-in functionality.
    > ---------------------------------------------------------
    >
    > I know, it is not so clever to throw things from a c'tor.

    There is absolutely nothong wrong about that. On the contrary: it is
    the only reasonable way to indicate failure of construction.
    [snip]

    /Peter
    peter koch, Nov 12, 2006
    #3
  4. Chameleon

    Salt_Peter Guest

    Chameleon wrote:
    > Is there a possibility to create memory leak, the code below if I run
    > the line:
    > ---------------------------------------------------------
    > MyClass cl = new MyClass();


    MyClass* p_cl = new MyClass;

    or even better:

    #include <boost/shared_ptr.hpp>
    boost::shared_ptr< MyClass > bsp( new MyClass );

    > ---------------------------------------------------------

    Why not:

    template< typename T >
    class MyClass
    {
    std::vector< T > va;
    std::vector< T > vb;
    public:
    MyClass() : va(), vb() { }
    MyClass(size_t);
    };

    > ---------------------------------------------------------
    > MyClass::MyClass()


    template< typename T >
    MyClass< T >::Myclass(size_t size) : va(size), vb(size)
    {
    }

    > {
    > try {
    > int *a = 0;
    > int *b = 0;
    > a = new int[X];
    > b = new int[X];
    > } catch (...) {
    > if (a) delete[] a;
    > if (b) delete[] b;
    > throw;
    > }
    > }
    > ---------------------------------------------------------
    >
    > I know, it is not so clever to throw things from a c'tor.


    Nothing wrong with throwing from a ctor, but not like this. catch
    explicitly using std::bad_alloc.

    >
    > But code above has any problem?


    Yes it does, if the second allocation generates a std::bad_alloc, you
    have a leak
    Thats why a smart pointer would be preferable.
    With vectors, none of this is needed.

    >
    >
    > thanks!
    >
    >
    > PS: I worked in C++ for 3 years but after I worked in Java for a 3 years
    > too. So the returning is a bit hmmm...
    Salt_Peter, Nov 12, 2006
    #4
  5. Chameleon

    Kai-Uwe Bux Guest

    Salt_Peter wrote:

    >
    > Chameleon wrote:
    >> Is there a possibility to create memory leak, the code below if I run
    >> the line:
    >> ---------------------------------------------------------
    >> MyClass cl = new MyClass();

    >
    > MyClass* p_cl = new MyClass;
    >
    > or even better:
    >
    > #include <boost/shared_ptr.hpp>
    > boost::shared_ptr< MyClass > bsp( new MyClass );
    >
    >> ---------------------------------------------------------

    > Why not:
    >
    > template< typename T >
    > class MyClass
    > {
    > std::vector< T > va;
    > std::vector< T > vb;
    > public:
    > MyClass() : va(), vb() { }
    > MyClass(size_t);
    > };
    >
    >> ---------------------------------------------------------
    >> MyClass::MyClass()

    >
    > template< typename T >
    > MyClass< T >::Myclass(size_t size) : va(size), vb(size)
    > {
    > }
    >
    >> {
    >> try {
    >> int *a = 0;
    >> int *b = 0;
    >> a = new int[X];
    >> b = new int[X];
    >> } catch (...) {
    >> if (a) delete[] a;
    >> if (b) delete[] b;
    >> throw;
    >> }
    >> }
    >> ---------------------------------------------------------
    >>
    >> I know, it is not so clever to throw things from a c'tor.

    >
    > Nothing wrong with throwing from a ctor, but not like this. catch
    > explicitly using std::bad_alloc.


    Why? Is there any observable difference?


    >>
    >> But code above has any problem?

    >
    > Yes it does, if the second allocation generates a std::bad_alloc, you
    > have a leak


    No: the catch-handler will be invoked in that case, too; and it will
    correctly handle the member array a.

    > Thats why a smart pointer would be preferable.


    Are you be refering to your first suggestion:

    shared_ptr< MyClass > bsp ( new MyClass );

    I think it would not help in a case where the constructor of MyClass leaks:
    what is leaked there cannot magically be recovered by the shared_ptr.


    > With vectors, none of this is needed.


    True.

    [snip]


    Kai-Uwe Bux
    Kai-Uwe Bux, Nov 12, 2006
    #5
  6. Chameleon

    Salt_Peter Guest

    Kai-Uwe Bux wrote:
    > Salt_Peter wrote:
    >
    > >
    > > Chameleon wrote:
    > >> Is there a possibility to create memory leak, the code below if I run
    > >> the line:
    > >> ---------------------------------------------------------
    > >> MyClass cl = new MyClass();

    > >
    > > MyClass* p_cl = new MyClass;
    > >
    > > or even better:
    > >
    > > #include <boost/shared_ptr.hpp>
    > > boost::shared_ptr< MyClass > bsp( new MyClass );
    > >
    > >> ---------------------------------------------------------

    > > Why not:
    > >
    > > template< typename T >
    > > class MyClass
    > > {
    > > std::vector< T > va;
    > > std::vector< T > vb;
    > > public:
    > > MyClass() : va(), vb() { }
    > > MyClass(size_t);
    > > };
    > >
    > >> ---------------------------------------------------------
    > >> MyClass::MyClass()

    > >
    > > template< typename T >
    > > MyClass< T >::Myclass(size_t size) : va(size), vb(size)
    > > {
    > > }
    > >
    > >> {
    > >> try {
    > >> int *a = 0;
    > >> int *b = 0;
    > >> a = new int[X];
    > >> b = new int[X];
    > >> } catch (...) {
    > >> if (a) delete[] a;
    > >> if (b) delete[] b;
    > >> throw;
    > >> }
    > >> }
    > >> ---------------------------------------------------------
    > >>
    > >> I know, it is not so clever to throw things from a c'tor.

    > >
    > > Nothing wrong with throwing from a ctor, but not like this. catch
    > > explicitly using std::bad_alloc.

    >
    > Why? Is there any observable difference?


    Well yes, from the point of view that the exception thrown is
    identifiable.

    >
    >
    > >>
    > >> But code above has any problem?

    > >
    > > Yes it does, if the second allocation generates a std::bad_alloc, you
    > > have a leak

    >
    > No: the catch-handler will be invoked in that case, too; and it will
    > correctly handle the member array a.


    Yes, thats right, if you handle the deallocation in the catch block as
    shown above.
    Sorry, my attention span is on the brink.

    >
    > > Thats why a smart pointer would be preferable.

    >
    > Are you be refering to your first suggestion:


    no, i was referring to a boost::shared_array for the members.

    >
    > shared_ptr< MyClass > bsp ( new MyClass );
    >
    > I think it would not help in a case where the constructor of MyClass leaks:
    > what is leaked there cannot magically be recovered by the shared_ptr.
    >
    >
    > > With vectors, none of this is needed.

    >
    > True.
    >
    > [snip]
    >
    >
    > Kai-Uwe Bux
    Salt_Peter, Nov 12, 2006
    #6
  7. Chameleon

    Old Wolf Guest

    Chameleon wrote:
    > Is there a possibility to create memory leak, the code below if I run
    > the line:
    > ---------------------------------------------------------
    > MyClass cl = new MyClass();
    > ---------------------------------------------------------
    > ???
    >
    > ---------------------------------------------------------
    > MyClass::MyClass()
    > {
    > try {
    > int *a = 0;
    > int *b = 0;
    > a = new int[X];
    > b = new int[X];
    > } catch (...) {
    > if (a) delete[] a;
    > if (b) delete[] b;
    > throw;
    > }
    > }


    The code doesn't even compile. "a" and "b" are local to the
    try block; they are not visible outside of those curly braces,
    in particular, from within the catch block.

    Perhaps you meant to do something such as :

    class MyClass
    {
    int a, b;
    MyClass(): a(0), b(0)
    {
    try {
    a = new int[X];
    b = new int[X];
    }
    etc.

    in which case the answer is that you will not have a memory
    leak because the catch block will free the memory.

    It is OK to throw exceptions from constructors; the rule is
    that an object is not considered to exist until after its
    constructor has completed, so the rest of the program
    (assuming you catch the exception) will behave as if the
    object never existed.

    If you're coming from Java, note that you almost never need
    to use "new" and "delete" in C++ ; you should use automatic
    memory management (eg. std::vector) wherever possible.
    Another Java-ism to avoid is:
    MyClass *cl = new MyClass();
    (note, the parentheses are redundant here). Better is:
    MyClass cl;
    and then the object will be automatically destroyed when it
    goes out of scope. Unless, of course, you want to manually
    manage the lifetime of cl.
    Old Wolf, Nov 12, 2006
    #7
  8. Chameleon

    drrn Guest

    > Perhaps you meant to do something such as :
    >
    > class MyClass
    > {
    > int a, b;
    > MyClass(): a(0), b(0)
    > {
    > try {
    > a = new int[X];
    > b = new int[X];
    > }
    > etc.


    An even better solution would be:

    class MyClass
    {
    int *a, *b;

    MyClass() : a( new int[X] ), b( new int[X] )
    {}
    ~MyClass() { delete[] a; delete[] b };
    };

    If either of the `new int[X]` calls fails then the memory will
    automagically be cleaned up. I might be mistaken, but I think I read
    that in TC++PL.
    drrn, Nov 13, 2006
    #8
  9. Chameleon

    Gavin Deane Guest

    drrn wrote:
    > > Perhaps you meant to do something such as :
    > >
    > > class MyClass
    > > {
    > > int a, b;
    > > MyClass(): a(0), b(0)
    > > {
    > > try {
    > > a = new int[X];
    > > b = new int[X];
    > > }
    > > etc.

    >
    > An even better solution would be:
    >
    > class MyClass
    > {
    > int *a, *b;
    >
    > MyClass() : a( new int[X] ), b( new int[X] )
    > {}
    > ~MyClass() { delete[] a; delete[] b };
    > };
    >
    > If either of the `new int[X]` calls fails then the memory will
    > automagically be cleaned up. I might be mistaken, but I think I read
    > that in TC++PL.


    You are mistaken. The problem in your code might well be exactly what
    the OP is trying to avoid. An object is not constructed until its
    constructor finishes executing. Using your code, if I do

    MyClass myObject;

    then, if b(new int[X]) throws, the construction of myObject has not yet
    finished so its destructor is not called during stack unwinding. The
    memory pointed to by the member variable a inside the partially
    constructed myObject is never freed.

    Gavin Deane
    Gavin Deane, Nov 13, 2006
    #9
  10. Salt_Peter wrote:
    > Kai-Uwe Bux wrote:
    >> Salt_Peter wrote:
    >>
    >>> Chameleon wrote:
    >>>> Is there a possibility to create memory leak, the code below if I run
    >>>> the line:
    >>>> ---------------------------------------------------------
    >>>> MyClass cl = new MyClass();
    >>> MyClass* p_cl = new MyClass;
    >>>
    >>> or even better:
    >>>
    >>> #include <boost/shared_ptr.hpp>
    >>> boost::shared_ptr< MyClass > bsp( new MyClass );
    >>>
    >>>> ---------------------------------------------------------
    >>> Why not:
    >>>
    >>> template< typename T >
    >>> class MyClass
    >>> {
    >>> std::vector< T > va;
    >>> std::vector< T > vb;
    >>> public:
    >>> MyClass() : va(), vb() { }
    >>> MyClass(size_t);
    >>> };
    >>>
    >>>> ---------------------------------------------------------
    >>>> MyClass::MyClass()
    >>> template< typename T >
    >>> MyClass< T >::Myclass(size_t size) : va(size), vb(size)
    >>> {
    >>> }
    >>>
    >>>> {
    >>>> try {
    >>>> int *a = 0;
    >>>> int *b = 0;
    >>>> a = new int[X];
    >>>> b = new int[X];
    >>>> } catch (...) {
    >>>> if (a) delete[] a;
    >>>> if (b) delete[] b;
    >>>> throw;
    >>>> }
    >>>> }
    >>>> ---------------------------------------------------------
    >>>>
    >>>> I know, it is not so clever to throw things from a c'tor.
    >>> Nothing wrong with throwing from a ctor, but not like this. catch
    >>> explicitly using std::bad_alloc.

    >> Why? Is there any observable difference?

    >
    > Well yes, from the point of view that the exception thrown is
    > identifiable.


    How does that change the behavior of the program in any way? In the case
    of int, no other exception could be thrown, but imagine if the code
    changes to use some other type. With the catch(...), the code will still
    function, and there will be no leak. If, on the other hand, you catch
    bad_alloc specifically, and some other exception is thrown, the code
    *will* leak.


    --
    Clark S. Cox III
    Clark S. Cox III, Nov 13, 2006
    #10
  11. drrn wrote:
    >> Perhaps you meant to do something such as :
    >>
    >> class MyClass
    >> {
    >> int a, b;
    >> MyClass(): a(0), b(0)
    >> {
    >> try {
    >> a = new int[X];
    >> b = new int[X];
    >> }
    >> etc.

    >
    > An even better solution would be:
    >
    > class MyClass
    > {
    > int *a, *b;
    >
    > MyClass() : a( new int[X] ), b( new int[X] )
    > {}
    > ~MyClass() { delete[] a; delete[] b };
    > };


    Or:

    class MyClass
    {
    std::vector<int> a, b;

    MyClass() : a(X), b(X) {}

    };

    --
    Clark S. Cox III
    Clark S. Cox III, Nov 13, 2006
    #11
  12. Chameleon

    Salt_Peter Guest

    drrn wrote:
    > > Perhaps you meant to do something such as :
    > >
    > > class MyClass
    > > {
    > > int a, b;
    > > MyClass(): a(0), b(0)
    > > {
    > > try {
    > > a = new int[X];
    > > b = new int[X];
    > > }
    > > etc.

    >
    > An even better solution would be:
    >
    > class MyClass
    > {
    > int *a, *b;
    >
    > MyClass() : a( new int[X] ), b( new int[X] )
    > {}
    > ~MyClass() { delete[] a; delete[] b };
    > };
    >
    > If either of the `new int[X]` calls fails then the memory will
    > automagically be cleaned up. I might be mistaken, but I think I read
    > that in TC++PL.


    No, the better solution involves using std::vector members.
    Salt_Peter, Nov 13, 2006
    #12
  13. Chameleon

    Salt_Peter Guest

    Clark S. Cox III wrote:
    > Salt_Peter wrote:
    > > Kai-Uwe Bux wrote:
    > >> Salt_Peter wrote:
    > >>
    > >>> Chameleon wrote:
    > >>>> Is there a possibility to create memory leak, the code below if I run
    > >>>> the line:
    > >>>> ---------------------------------------------------------
    > >>>> MyClass cl = new MyClass();
    > >>> MyClass* p_cl = new MyClass;
    > >>>
    > >>> or even better:
    > >>>
    > >>> #include <boost/shared_ptr.hpp>
    > >>> boost::shared_ptr< MyClass > bsp( new MyClass );
    > >>>
    > >>>> ---------------------------------------------------------
    > >>> Why not:
    > >>>
    > >>> template< typename T >
    > >>> class MyClass
    > >>> {
    > >>> std::vector< T > va;
    > >>> std::vector< T > vb;
    > >>> public:
    > >>> MyClass() : va(), vb() { }
    > >>> MyClass(size_t);
    > >>> };
    > >>>
    > >>>> ---------------------------------------------------------
    > >>>> MyClass::MyClass()
    > >>> template< typename T >
    > >>> MyClass< T >::Myclass(size_t size) : va(size), vb(size)
    > >>> {
    > >>> }
    > >>>
    > >>>> {
    > >>>> try {
    > >>>> int *a = 0;
    > >>>> int *b = 0;
    > >>>> a = new int[X];
    > >>>> b = new int[X];
    > >>>> } catch (...) {
    > >>>> if (a) delete[] a;
    > >>>> if (b) delete[] b;
    > >>>> throw;
    > >>>> }
    > >>>> }
    > >>>> ---------------------------------------------------------
    > >>>>
    > >>>> I know, it is not so clever to throw things from a c'tor.
    > >>> Nothing wrong with throwing from a ctor, but not like this. catch
    > >>> explicitly using std::bad_alloc.
    > >> Why? Is there any observable difference?

    > >
    > > Well yes, from the point of view that the exception thrown is
    > > identifiable.

    >
    > How does that change the behavior of the program in any way? In the case
    > of int, no other exception could be thrown, but imagine if the code
    > changes to use some other type. With the catch(...), the code will still
    > function, and there will be no leak. If, on the other hand, you catch
    > bad_alloc specifically, and some other exception is thrown, the code
    > *will* leak.
    >


    One would catch a const std::exception&, not bad_alloc.
    I'm not saying that would be "better" either.
    There is nothing wrong with catch(...) since one can rethrow a relevent
    exception.
    All i'm saying is that somewhere you'll want to know that a bad_alloc
    happened.
    So as not to have to investigate an issue that may only occur under
    certain extraneous conditions.
    Salt_Peter, Nov 13, 2006
    #13
  14. Chameleon

    Salt_Peter Guest

    Clark S. Cox III wrote:
    > drrn wrote:
    > >> Perhaps you meant to do something such as :
    > >>
    > >> class MyClass
    > >> {
    > >> int a, b;
    > >> MyClass(): a(0), b(0)
    > >> {
    > >> try {
    > >> a = new int[X];
    > >> b = new int[X];
    > >> }
    > >> etc.

    > >
    > > An even better solution would be:
    > >
    > > class MyClass
    > > {
    > > int *a, *b;
    > >
    > > MyClass() : a( new int[X] ), b( new int[X] )
    > > {}
    > > ~MyClass() { delete[] a; delete[] b };
    > > };

    >
    > Or:
    >
    > class MyClass
    > {
    > std::vector<int> a, b;
    >
    > MyClass() : a(X), b(X) {}


    public:
    MyClass(size_t X) : a(X), b(X) { }
    >
    > };
    >


    And yes, the std::vector is both the correct solution and a much more
    usefull solution.
    Salt_Peter, Nov 13, 2006
    #14
  15. Chameleon

    Kai-Uwe Bux Guest

    Salt_Peter wrote:

    >
    > Clark S. Cox III wrote:
    >> Salt_Peter wrote:
    >> > Kai-Uwe Bux wrote:
    >> >> Salt_Peter wrote:
    >> >>
    >> >>> Chameleon wrote:
    >> >>>> Is there a possibility to create memory leak, the code below if I
    >> >>>> run the line:
    >> >>>> ---------------------------------------------------------
    >> >>>> MyClass cl = new MyClass();
    >> >>> MyClass* p_cl = new MyClass;
    >> >>>
    >> >>> or even better:
    >> >>>
    >> >>> #include <boost/shared_ptr.hpp>
    >> >>> boost::shared_ptr< MyClass > bsp( new MyClass );
    >> >>>
    >> >>>> ---------------------------------------------------------
    >> >>> Why not:
    >> >>>
    >> >>> template< typename T >
    >> >>> class MyClass
    >> >>> {
    >> >>> std::vector< T > va;
    >> >>> std::vector< T > vb;
    >> >>> public:
    >> >>> MyClass() : va(), vb() { }
    >> >>> MyClass(size_t);
    >> >>> };
    >> >>>
    >> >>>> ---------------------------------------------------------
    >> >>>> MyClass::MyClass()
    >> >>> template< typename T >
    >> >>> MyClass< T >::Myclass(size_t size) : va(size), vb(size)
    >> >>> {
    >> >>> }
    >> >>>
    >> >>>> {
    >> >>>> try {
    >> >>>> int *a = 0;
    >> >>>> int *b = 0;
    >> >>>> a = new int[X];
    >> >>>> b = new int[X];
    >> >>>> } catch (...) {
    >> >>>> if (a) delete[] a;
    >> >>>> if (b) delete[] b;
    >> >>>> throw;
    >> >>>> }
    >> >>>> }
    >> >>>> ---------------------------------------------------------
    >> >>>>
    >> >>>> I know, it is not so clever to throw things from a c'tor.
    >> >>> Nothing wrong with throwing from a ctor, but not like this. catch
    >> >>> explicitly using std::bad_alloc.
    >> >> Why? Is there any observable difference?
    >> >
    >> > Well yes, from the point of view that the exception thrown is
    >> > identifiable.

    >>
    >> How does that change the behavior of the program in any way? In the case
    >> of int, no other exception could be thrown, but imagine if the code
    >> changes to use some other type. With the catch(...), the code will still
    >> function, and there will be no leak. If, on the other hand, you catch
    >> bad_alloc specifically, and some other exception is thrown, the code
    >> *will* leak.
    >>

    >
    > One would catch a const std::exception&, not bad_alloc.


    You really would want to catch(...). For instance, if int was replaced by a
    template type parameter, anything could be thrown. There is no guarantee
    that a client provided type would not throw an int or some such thing.


    > I'm not saying that would be "better" either.
    > There is nothing wrong with catch(...) since one can rethrow a relevent
    > exception.
    > All i'm saying is that somewhere you'll want to know that a bad_alloc
    > happened.


    Maybe, but where? In the above code, you need to deallocate a and b
    anyway, regardless of the reason for why their construction failed. Any
    more specific analysis can be postponed (and that is why the catch-handler
    rethrows).

    > So as not to have to investigate an issue that may only occur under
    > certain extraneous conditions.


    Huh?


    Best

    Kai-Uwe Bux
    Kai-Uwe Bux, Nov 13, 2006
    #15
  16. Chameleon

    Salt_Peter Guest

    Kai-Uwe Bux wrote:
    > Salt_Peter wrote:
    >
    > >
    > > Clark S. Cox III wrote:
    > >> Salt_Peter wrote:
    > >> > Kai-Uwe Bux wrote:
    > >> >> Salt_Peter wrote:
    > >> >>
    > >> >>> Chameleon wrote:
    > >> >>>> Is there a possibility to create memory leak, the code below if I
    > >> >>>> run the line:
    > >> >>>> ---------------------------------------------------------
    > >> >>>> MyClass cl = new MyClass();
    > >> >>> MyClass* p_cl = new MyClass;
    > >> >>>
    > >> >>> or even better:
    > >> >>>
    > >> >>> #include <boost/shared_ptr.hpp>
    > >> >>> boost::shared_ptr< MyClass > bsp( new MyClass );
    > >> >>>
    > >> >>>> ---------------------------------------------------------
    > >> >>> Why not:
    > >> >>>
    > >> >>> template< typename T >
    > >> >>> class MyClass
    > >> >>> {
    > >> >>> std::vector< T > va;
    > >> >>> std::vector< T > vb;
    > >> >>> public:
    > >> >>> MyClass() : va(), vb() { }
    > >> >>> MyClass(size_t);
    > >> >>> };
    > >> >>>
    > >> >>>> ---------------------------------------------------------
    > >> >>>> MyClass::MyClass()
    > >> >>> template< typename T >
    > >> >>> MyClass< T >::Myclass(size_t size) : va(size), vb(size)
    > >> >>> {
    > >> >>> }
    > >> >>>
    > >> >>>> {
    > >> >>>> try {
    > >> >>>> int *a = 0;
    > >> >>>> int *b = 0;
    > >> >>>> a = new int[X];
    > >> >>>> b = new int[X];
    > >> >>>> } catch (...) {
    > >> >>>> if (a) delete[] a;
    > >> >>>> if (b) delete[] b;
    > >> >>>> throw;
    > >> >>>> }
    > >> >>>> }
    > >> >>>> ---------------------------------------------------------
    > >> >>>>
    > >> >>>> I know, it is not so clever to throw things from a c'tor.
    > >> >>> Nothing wrong with throwing from a ctor, but not like this. catch
    > >> >>> explicitly using std::bad_alloc.
    > >> >> Why? Is there any observable difference?
    > >> >
    > >> > Well yes, from the point of view that the exception thrown is
    > >> > identifiable.
    > >>
    > >> How does that change the behavior of the program in any way? In the case
    > >> of int, no other exception could be thrown, but imagine if the code
    > >> changes to use some other type. With the catch(...), the code will still
    > >> function, and there will be no leak. If, on the other hand, you catch
    > >> bad_alloc specifically, and some other exception is thrown, the code
    > >> *will* leak.
    > >>

    > >
    > > One would catch a const std::exception&, not bad_alloc.

    >
    > You really would want to catch(...). For instance, if int was replaced by a
    > template type parameter, anything could be thrown. There is no guarantee
    > that a client provided type would not throw an int or some such thing.


    Precisely my point.
    Then consider:
    try
    {
    // do whatever
    }
    catch( const std::exception& r_e)
    {
    std::cerr << "error: " << r_e.what() << std::endl;
    // rethrow?
    }
    catch(...)
    {
    std::cerr << "error: unexpected" << std::endl;
    // rethrow??
    }

    Forget the deallocation for now. The point here is to give yourself a
    chance to figure out what exception is being caught.

    >
    >
    > > I'm not saying that would be "better" either.
    > > There is nothing wrong with catch(...) since one can rethrow a relevent
    > > exception.
    > > All i'm saying is that somewhere you'll want to know that a bad_alloc
    > > happened.

    >
    > Maybe, but where? In the above code, you need to deallocate a and b
    > anyway, regardless of the reason for why their construction failed. Any
    > more specific analysis can be postponed (and that is why the catch-handler
    > rethrows).


    Where else - in main. Is it not warranted to keep the whole code
    exception safe? - not just a lonely ctor?

    >
    > > So as not to have to investigate an issue that may only occur under
    > > certain extraneous conditions.

    >
    > Huh?


    So your program just crashed without a hint on what was thrown, thats
    the Huh? You are assuming that a bad_alloc caused it, but thats not
    neccessarily so. Relying on the catch(..) block alone got you there.
    Salt_Peter, Nov 13, 2006
    #16
  17. Chameleon

    Kai-Uwe Bux Guest

    Salt_Peter wrote:

    >
    > Kai-Uwe Bux wrote:
    >> Salt_Peter wrote:
    >>
    >> >
    >> > Clark S. Cox III wrote:
    >> >> Salt_Peter wrote:
    >> >> > Kai-Uwe Bux wrote:
    >> >> >> Salt_Peter wrote:
    >> >> >>
    >> >> >>> Chameleon wrote:
    >> >> >>>> Is there a possibility to create memory leak, the code below if I
    >> >> >>>> run the line:
    >> >> >>>> ---------------------------------------------------------
    >> >> >>>> MyClass cl = new MyClass();
    >> >> >>> MyClass* p_cl = new MyClass;
    >> >> >>>
    >> >> >>> or even better:
    >> >> >>>
    >> >> >>> #include <boost/shared_ptr.hpp>
    >> >> >>> boost::shared_ptr< MyClass > bsp( new MyClass );
    >> >> >>>
    >> >> >>>> ---------------------------------------------------------
    >> >> >>> Why not:
    >> >> >>>
    >> >> >>> template< typename T >
    >> >> >>> class MyClass
    >> >> >>> {
    >> >> >>> std::vector< T > va;
    >> >> >>> std::vector< T > vb;
    >> >> >>> public:
    >> >> >>> MyClass() : va(), vb() { }
    >> >> >>> MyClass(size_t);
    >> >> >>> };
    >> >> >>>
    >> >> >>>> ---------------------------------------------------------
    >> >> >>>> MyClass::MyClass()
    >> >> >>> template< typename T >
    >> >> >>> MyClass< T >::Myclass(size_t size) : va(size), vb(size)
    >> >> >>> {
    >> >> >>> }
    >> >> >>>
    >> >> >>>> {
    >> >> >>>> try {
    >> >> >>>> int *a = 0;
    >> >> >>>> int *b = 0;
    >> >> >>>> a = new int[X];
    >> >> >>>> b = new int[X];
    >> >> >>>> } catch (...) {
    >> >> >>>> if (a) delete[] a;
    >> >> >>>> if (b) delete[] b;
    >> >> >>>> throw;
    >> >> >>>> }
    >> >> >>>> }
    >> >> >>>> ---------------------------------------------------------
    >> >> >>>>
    >> >> >>>> I know, it is not so clever to throw things from a c'tor.
    >> >> >>> Nothing wrong with throwing from a ctor, but not like this. catch
    >> >> >>> explicitly using std::bad_alloc.
    >> >> >> Why? Is there any observable difference?
    >> >> >
    >> >> > Well yes, from the point of view that the exception thrown is
    >> >> > identifiable.
    >> >>
    >> >> How does that change the behavior of the program in any way? In the
    >> >> case of int, no other exception could be thrown, but imagine if the
    >> >> code changes to use some other type. With the catch(...), the code
    >> >> will still function, and there will be no leak. If, on the other hand,
    >> >> you catch bad_alloc specifically, and some other exception is thrown,
    >> >> the code *will* leak.
    >> >>
    >> >
    >> > One would catch a const std::exception&, not bad_alloc.

    >>
    >> You really would want to catch(...). For instance, if int was replaced by
    >> a template type parameter, anything could be thrown. There is no
    >> guarantee that a client provided type would not throw an int or some such
    >> thing.

    >
    > Precisely my point.
    > Then consider:
    > try
    > {
    > // do whatever
    > }
    > catch( const std::exception& r_e)
    > {
    > std::cerr << "error: " << r_e.what() << std::endl;
    > // rethrow?
    > }
    > catch(...)
    > {
    > std::cerr << "error: unexpected" << std::endl;
    > // rethrow??
    > }
    >
    > Forget the deallocation for now. The point here is to give yourself a
    > chance to figure out what exception is being caught.


    You seem to think, that the code of the OP does not provide such a chance.

    >>
    >>
    >> > I'm not saying that would be "better" either.
    >> > There is nothing wrong with catch(...) since one can rethrow a relevent
    >> > exception.
    >> > All i'm saying is that somewhere you'll want to know that a bad_alloc
    >> > happened.

    >>
    >> Maybe, but where? In the above code, you need to deallocate a and b
    >> anyway, regardless of the reason for why their construction failed. Any
    >> more specific analysis can be postponed (and that is why the
    >> catch-handler rethrows).

    >
    > Where else - in main. Is it not warranted to keep the whole code
    > exception safe? - not just a lonely ctor?
    >
    >>
    >> > So as not to have to investigate an issue that may only occur under
    >> > certain extraneous conditions.

    >>
    >> Huh?

    >
    > So your program just crashed without a hint on what was thrown, thats
    > the Huh? You are assuming that a bad_alloc caused it, but thats not
    > neccessarily so. Relying on the catch(..) block alone got you there.


    You are missing the point. Consider the templated variation:

    template < typename T >
    struct X {
    T * a;
    T * b;
    std::size_t l;
    X ( std::size_t n )
    : a ( 0 )
    , b ( 0 )
    , l ( n )
    {
    try{
    a = new T [l];
    b = new T [l];
    }
    catch( ... ) {
    delete a;
    delete b;
    throw;
    }
    }
    };

    Within the constructor, you have no chance of knowing what causes the
    exception (you might guess it's bad_alloc, but it could be the constructor
    of T just as well). Also, the constructor of X does not really want to
    know. All it can do is clean up the mess it did during construction, i.e.,
    deallocate the members a and b. For that purpose, it does not need to
    know why construction of these arrays failed. Thus, the catch(...) in the
    constructor of X is justified.

    Only the client, who knows what T is can know what T might throw. Therefore,
    the client should do:

    try {
    X< MyConcreteType > x ( 23 );
    }
    catch( std::bad_alloc & ) {
    }
    catch( whatever ) {
    }

    The point is that the constructor of X is simply not the right place for
    specifying which exception to catch.

    Now, I will grant you that one could pass a little more detailed information
    to the ambient context so that it can distinguish whether a bad_alloc was
    thrown from X of from MyConcreteType. However, I do not quite see how to
    make sensible use of that additional piece of information.


    Best

    Kai-Uwe Bux
    Kai-Uwe Bux, Nov 13, 2006
    #17
  18. Chameleon

    VJ Guest

    Kai-Uwe Bux wrote:
    >
    > You are missing the point. Consider the templated variation:
    >
    > template < typename T >
    > struct X {
    > T * a;
    > T * b;
    > std::size_t l;
    > X ( std::size_t n )
    > : a ( 0 )
    > , b ( 0 )
    > , l ( n )
    > {
    > try{
    > a = new T [l];
    > b = new T [l];
    > }
    > catch( ... ) {
    > delete a;
    > delete b;
    > throw;
    > }
    > }
    > };
    >
    > Within the constructor, you have no chance of knowing what causes the
    > exception (you might guess it's bad_alloc, but it could be the constructor
    > of T just as well). Also, the constructor of X does not really want to
    > know. All it can do is clean up the mess it did during construction, i.e.,
    > deallocate the members a and b. For that purpose, it does not need to
    > know why construction of these arrays failed. Thus, the catch(...) in the
    > constructor of X is justified.


    In this specific code - I see only std::bad_alloc could be thrown


    >
    > Only the client, who knows what T is can know what T might throw. Therefore,
    > the client should do:
    >
    > try {
    > X< MyConcreteType > x ( 23 );
    > }
    > catch( std::bad_alloc & ) {
    > }
    > catch( whatever ) {
    > }
    >
    > The point is that the constructor of X is simply not the right place for
    > specifying which exception to catch.


    It is if you want to throw specific exception. For example


    class S
    {
    public:
    class P{};

    S()
    {
    throw P();
    }
    };




    >
    > Now, I will grant you that one could pass a little more detailed information
    > to the ambient context so that it can distinguish whether a bad_alloc was
    > thrown from X of from MyConcreteType. However, I do not quite see how to
    > make sensible use of that additional piece of information.


    In this example no, but there might be a function called in the ctor,
    which throws another exception. If you catch that exception, who knows
    what you can break in you program, or you can hide a bug using catch(...)

    Using catch all is bad for next reason:

    "Exception handling is often used as the lazy way out of a problem. Not
    sure exactly why something is failing intermittently? Don't bother
    investigating, just trap the error and keep going. If it's important,
    someone else can fix it properly later."

    but it has some uses

    This is from
    http://neil.fraser.name/writing/exception/
    I find this text is fun and useful :)
    VJ, Nov 13, 2006
    #18
  19. Chameleon

    Kai-Uwe Bux Guest

    VJ wrote:

    > Kai-Uwe Bux wrote:
    >>
    >> You are missing the point. Consider the templated variation:
    >>
    >> template < typename T >
    >> struct X {
    >> T * a;
    >> T * b;
    >> std::size_t l;
    >> X ( std::size_t n )
    >> : a ( 0 )
    >> , b ( 0 )
    >> , l ( n )
    >> {
    >> try{
    >> a = new T [l];
    >> b = new T [l];
    >> }
    >> catch( ... ) {
    >> delete a;
    >> delete b;
    >> throw;
    >> }
    >> }
    >> };
    >>
    >> Within the constructor, you have no chance of knowing what causes the
    >> exception (you might guess it's bad_alloc, but it could be the
    >> constructor of T just as well). Also, the constructor of X does not
    >> really want to know. All it can do is clean up the mess it did during
    >> construction, i.e.,
    >> deallocate the members a and b. For that purpose, it does not need to
    >> know why construction of these arrays failed. Thus, the catch(...) in the
    >> constructor of X is justified.

    >
    > In this specific code - I see only std::bad_alloc could be thrown


    Wrong: the lines

    a = new T [l];
    b = new T [l];

    attempt to not only allocate memory for two arrays (which could fail
    throwing the bad_alloc) but also to construct 2l objects of type T. If the
    constructor of T for one of these 2l object throws something, then this
    something will be caught and rethrown in the catch handler.

    [snip]

    >> Now, I will grant you that one could pass a little more detailed
    >> information to the ambient context so that it can distinguish whether a
    >> bad_alloc was thrown from X of from MyConcreteType. However, I do not
    >> quite see how to make sensible use of that additional piece of
    >> information.

    >
    > In this example no, but there might be a function called in the ctor,
    > which throws another exception. If you catch that exception, who knows
    > what you can break in you program, or you can hide a bug using catch(...)


    True: in a different code in the try-block, catch(...) could be bad. Note
    that nobody in this thread says that catch(...) is always or even generally
    a good idea. We are talking about very specific code.

    [snip]



    Best

    Kai-Uwe Bux
    Kai-Uwe Bux, Nov 13, 2006
    #19
  20. VJ wrote:
    > Kai-Uwe Bux wrote:
    >>
    >> You are missing the point. Consider the templated variation:
    >>
    >> template < typename T >
    >> struct X {
    >> T * a;
    >> T * b;
    >> std::size_t l;
    >> X ( std::size_t n )
    >> : a ( 0 )
    >> , b ( 0 )
    >> , l ( n )
    >> {
    >> try{
    >> a = new T [l];
    >> b = new T [l];
    >> }
    >> catch( ... ) {
    >> delete a;
    >> delete b;
    >> throw;
    >> }
    >> }
    >> };
    >>
    >> Within the constructor, you have no chance of knowing what causes the
    >> exception (you might guess it's bad_alloc, but it could be the
    >> constructor
    >> of T just as well). Also, the constructor of X does not really want to
    >> know. All it can do is clean up the mess it did during construction,
    >> i.e.,
    >> deallocate the members a and b. For that purpose, it does not need to
    >> know why construction of these arrays failed. Thus, the catch(...) in the
    >> constructor of X is justified.

    >
    > In this specific code - I see only std::bad_alloc could be thrown


    Really? What if MyConcreteType is defined like so:

    struct MyConcreteType
    {
    MyConcreteType() { throw 42; }
    };

    >>
    >> Only the client, who knows what T is can know what T might throw.
    >> Therefore,
    >> the client should do:
    >>
    >> try {
    >> X< MyConcreteType > x ( 23 );
    >> }
    >> catch( std::bad_alloc & ) {
    >> }
    >> catch( whatever ) {
    >> }
    >>
    >> The point is that the constructor of X is simply not the right place for
    >> specifying which exception to catch.

    >
    > It is if you want to throw specific exception. For example
    >
    >
    > class S
    > {
    > public:
    > class P{};
    >
    > S()
    > {
    > throw P();
    > }
    > };
    >
    >
    >
    >
    >>
    >> Now, I will grant you that one could pass a little more detailed
    >> information
    >> to the ambient context so that it can distinguish whether a bad_alloc was
    >> thrown from X of from MyConcreteType. However, I do not quite see how to
    >> make sensible use of that additional piece of information.

    >
    > In this example no, but there might be a function called in the ctor,
    > which throws another exception. If you catch that exception, who knows
    > what you can break in you program,


    Tell me one thing that could be broken by this use of "catch(...)".

    > or you can hide a bug using catch(...)


    Not if you rethrow it. In this constructor the *only* reaswon for
    catching an exception is to delete the pointers and rethrow. In this
    case, *every* exception should be treated in exactly the same manner.
    When that is true (i.e. that the response to every exception is the
    same) then catch(...) is the correct response.

    > Using catch all is bad for next reason:
    >
    > "Exception handling is often used as the lazy way out of a problem. Not
    > sure exactly why something is failing intermittently? Don't bother
    > investigating, just trap the error and keep going. If it's important,
    > someone else can fix it properly later."
    >
    > but it has some uses
    >
    > This is from
    > http://neil.fraser.name/writing/exception/
    > I find this text is fun and useful :)



    --
    Clark S. Cox III
    Clark S. Cox III, Nov 13, 2006
    #20
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Kerri
    Replies:
    2
    Views:
    13,023
    Kevin Spencer
    Oct 27, 2003
  2. =?Utf-8?B?T3Jlbg==?=

    C'tor of WebUserControl

    =?Utf-8?B?T3Jlbg==?=, Oct 16, 2005, in forum: ASP .Net
    Replies:
    2
    Views:
    465
    =?Utf-8?B?T3Jlbg==?=
    Oct 17, 2005
  3. Replies:
    15
    Views:
    7,532
    Roedy Green
    Sep 8, 2005
  4. Chameleon

    new and throw in c'tor

    Chameleon, Jan 10, 2007, in forum: C++
    Replies:
    3
    Views:
    305
    Peter
    Jan 10, 2007
  5. Emanuele D'Arrigo

    To throw or to throw not?

    Emanuele D'Arrigo, Nov 14, 2008, in forum: Python
    Replies:
    6
    Views:
    319
    Emanuele D'Arrigo
    Nov 15, 2008
Loading...

Share This Page