Challenging GotW 66's moral

Discussion in 'C++' started by George2, Dec 27, 2007.

  1. George2

    George2 Guest

    Hello everyone,


    In GotW #66, one of the moral is the exception handler of constructor
    should not do any like resource free task. I do not agree. Here is the
    quoated moral and my code to prove this moral will have memory leak.

    Anything wrong with my analysis?

    http://www.gotw.ca/gotw/066.htm

    Moral #1: Constructor function-try-block handlers have only one
    purpose -- to translate an exception. (And maybe to do logging or some
    other side effects.) They are not useful for any other purpose.


    Code:
    class A
    {
    private:
    
    int* p;
    
    public:
    
        A()
        try
        {
            p = new int[10];
    
            // there are some other exceptions here
    
        }
        catch (bad_alloc)
        {
            // do not delete since bad_alloc means memory pointed by p is
    not allocated
        }
        catch (...)
        {
            // if we do not delete p, there will be memory leak
            // at this point, we are conflicting with Gotw 66's moral 1
            if (p) delete[] p;
        }
    }
    

    thanks in advance,
    George
    George2, Dec 27, 2007
    #1
    1. Advertising

  2. George2

    Salt_Peter Guest

    On Dec 27, 1:54 am, George2 <> wrote:
    > Hello everyone,
    >
    > In GotW #66, one of the moral is the exception handler of constructor
    > should not do any like resource free task. I do not agree. Here is the
    > quoated moral and my code to prove this moral will have memory leak.
    >
    > Anything wrong with my analysis?
    >
    > http://www.gotw.ca/gotw/066.htm
    >
    > Moral #1: Constructor function-try-block handlers have only one
    > purpose -- to translate an exception. (And maybe to do logging or some
    > other side effects.) They are not useful for any other purpose.
    >
    >
    Code:
    > class A
    > {
    > private:
    >
    > int* p;
    >
    > public:
    >
    >     A()
    >     try
    >     {
    >         p = new int[10];
    >
    >         // there are some other exceptions here
    >
    >     }
    >     catch (bad_alloc)
    >     {
    >         // do not delete since bad_alloc means memory pointed by p is
    > not allocated
    >     }
    >     catch (...)
    >     {
    >         // if we do not delete p, there will be memory leak
    >         // at this point, we are conflicting with Gotw 66's moral 1
    >         if (p) delete[] p;[/color]
    
    at this point, p would never be 0 (even if the new allocation in ctor
    body was not processed yet).
    Pointer p, as is, has either a valid address or garbage in it.
    As far as that catch block is concerned, it'll always delete [] p.
    
    So how about:
    
    A() : p(0)
    try
    {
      p = new int[10];
    }
    catch(...)
    {
      if (p) delete[] p;
    }
    
    have you considered shared_ptr instead of doing decadent new
    allocations?
    Salt_Peter, Dec 27, 2007
    #2
    1. Advertising

  3. On 2007-12-27 07:54, George2 wrote:
    > Hello everyone,
    >
    >
    > In GotW #66, one of the moral is the exception handler of constructor
    > should not do any like resource free task. I do not agree. Here is the
    > quoated moral and my code to prove this moral will have memory leak.
    >
    > Anything wrong with my analysis?
    >
    > http://www.gotw.ca/gotw/066.htm
    >
    > Moral #1: Constructor function-try-block handlers have only one
    > purpose -- to translate an exception. (And maybe to do logging or some
    > other side effects.) They are not useful for any other purpose.
    >
    >
    >
    Code:
    > class A
    > {
    > private:
    > 
    > int* p;
    > 
    > public:
    > 
    >     A()
    >     try
    >     {
    >         p = new int[10];
    > 
    >         // there are some other exceptions here
    > 
    >     }
    >     catch (bad_alloc)
    >     {
    >         // do not delete since bad_alloc means memory pointed by p is
    > not allocated
    >     }
    >     catch (...)
    >     {
    >         // if we do not delete p, there will be memory leak
    >         // at this point, we are conflicting with Gotw 66's moral 1
    >         if (p) delete[] p;
    >     }
    > }
    > 


    The advice assumes that you follow other advices, such as using RAII and
    writing exception safe code, if you did you would never end up in a
    situation where you need to free any memory in a catch-block. One
    example of that would be to replace p with a smart-pointer.

    --
    Erik Wikström
    Erik Wikström, Dec 27, 2007
    #3
  4. George2

    Daniel T. Guest

    George2 <> wrote:

    > Hello everyone,
    >
    >
    > In GotW #66, one of the moral is the exception handler of constructor
    > should not do any like resource free task. I do not agree. Here is the
    > quoated moral and my code to prove this moral will have memory leak.
    >
    > Anything wrong with my analysis?


    Your idea is covered in the article:

    "--But wait!" I hear someone interrupting from the middle of the
    room. "I don't agree with Moral #1. I can think of another possible
    use for constructor function-try-blocks, namely to free resources
    allocated in the initializer list or in the constructor body!"

    Sorry, nope. After all, remember that once you get into your
    constructor try-block's handler, any local variables in the
    constructor body are also already out of scope, and you are
    guaranteed that no base subobjects or member objects exist any more,
    period. You can't even refer to their names.

    Maybe the output to the following will help:

    class B {
    public:
    B() { cout << "B()\n"; }
    ~B() { cout << "~B()\n"; }
    void foo() { cout << "B::foo()\n"; }
    };

    class A
    {
    private:
    B b;
    public:
    A() try: b()
    {
    throw -1;
    }
    catch (...)
    {
    b.foo();
    }
    };

    int main() {
    try {
    A a;
    }
    catch ( ... ) { }
    }

    Note that B::foo() is called *after b's destructor has already been
    called.* Thus invoking undefined behavior.

    > http://www.gotw.ca/gotw/066.htm
    >
    > Moral #1: Constructor function-try-block handlers have only one
    > purpose -- to translate an exception. (And maybe to do logging or some
    > other side effects.) They are not useful for any other purpose.
    >
    >
    >
    Code:
    > class A
    > {
    > private:
    > 
    > int* p;
    > 
    > public:
    > 
    >     A()
    >     try
    >     {
    >         p = new int[10];
    > 
    >         // there are some other exceptions here
    > 
    >     }
    >     catch (bad_alloc)
    >     {
    >         // do not delete since bad_alloc means memory pointed by p is
    > not allocated
    >     }
    >     catch (...)
    >     {
    >         // if we do not delete p, there will be memory leak
    >         // at this point, we are conflicting with Gotw 66's moral 1
    >         if (p) delete[] p;[/color]
    
    'p' doesn't exist once you get in the catch block. Yes, in your example 
    it happens to still point to the right place, but there is no guarantee 
    that this is true. Frankly, I'm surprised the code even compiled.
    [color=blue]
    >     }
    > }
    > 
    >
    >
    > thanks in advance,
    > George
    Daniel T., Dec 27, 2007
    #4
  5. George2

    Salt_Peter Guest

    On Dec 27, 4:58 am, Salt_Peter <> wrote:
    > On Dec 27, 1:54 am, George2 <> wrote:
    >
    >
    >
    > > Hello everyone,

    >
    > > In GotW #66, one of the moral is the exception handler of constructor
    > > should not do any like resource free task. I do not agree. Here is the
    > > quoated moral and my code to prove this moral will have memory leak.

    >
    > > Anything wrong with my analysis?

    >
    > >http://www.gotw.ca/gotw/066.htm

    >
    > > Moral #1: Constructor function-try-block handlers have only one
    > > purpose -- to translate an exception. (And maybe to do logging or some
    > > other side effects.) They are not useful for any other purpose.

    >
    > >
    Code:
    > > class A
    > > {
    > > private:[/color]
    >[color=green]
    > > int* p;[/color]
    >[color=green]
    > > public:[/color]
    >[color=green]
    > >     A()
    > >     try
    > >     {
    > >         p = new int[10];[/color]
    >[color=green]
    > >         // there are some other exceptions here[/color]
    >[color=green]
    > >     }
    > >     catch (bad_alloc)
    > >     {
    > >         // do not delete since bad_alloc means memory pointed by p is
    > > not allocated
    > >     }
    > >     catch (...)
    > >     {
    > >         // if we do not delete p, there will be memory leak
    > >         // at this point, we are conflicting with Gotw 66's moral 1
    > >         if (p) delete[] p;[/color]
    >
    > at this point, p would never be 0 (even if the new allocation in ctor
    > body was not processed yet).
    > Pointer p, as is, has either a valid address or garbage in it.
    > As far as that catch block is concerned, it'll always delete [] p.
    >
    > So how about:
    >
    > A() : p(0)
    > try
    > {
    >   p = new int[10];}
    >
    > catch(...)
    > {
    >   if (p) delete[] p;
    >
    > }
    >
    > have you considered shared_ptr instead of doing decadent new
    > allocations?[/color]
    
    
    obviously, clarification is required since you can't allocate an array
    using boost::shared_ptr.
    replace the array with a std:vector or use boost::scoped_array.
    Salt_Peter, Dec 27, 2007
    #5
    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. =?Utf-8?B?cmFtYXRh?=

    Classical Complex challenging Asp and SQL problem

    =?Utf-8?B?cmFtYXRh?=, Apr 30, 2005, in forum: ASP .Net
    Replies:
    3
    Views:
    449
    Mark Rae
    May 3, 2005
  2. Srini
    Replies:
    5
    Views:
    292
    Herb Sutter
    Sep 10, 2005
  3. Filimon Roukoutakis

    Is this valid and moral C++?

    Filimon Roukoutakis, Mar 18, 2007, in forum: C++
    Replies:
    30
    Views:
    977
    Richard Herring
    Apr 3, 2007
  4. Niels Dekker - no return address

    GotW #88: Is it safe to const_cast a reference to a temporary?

    Niels Dekker - no return address, Feb 2, 2008, in forum: C++
    Replies:
    17
    Views:
    884
    Niels Dekker - no return address
    Feb 6, 2008
  5. puzzlecracker

    stock unwinding from GotW #47

    puzzlecracker, May 1, 2008, in forum: C++
    Replies:
    0
    Views:
    309
    puzzlecracker
    May 1, 2008
Loading...

Share This Page