CODEWITHSUNDEEP
c++exception
Manish Shukla
Manish Shukla

Reputation: 562

Confusion on C++ programming practise with exception handling

I have a C++ code mentioned below:

#include<iostream>
#include<stdexcept>

const long MAX = 10240000;

class Widget{
      public:
             Widget(){
                      ok = new int [1024];
                      prob = new int [100*MAX];
             }
             ~Widget(){
                       std::cout<<"\nDtoR of Widget is called\n";
                       delete ok; ok = NULL;
                       delete prob; prob = NULL;
             }
             //keeping below public: Intentionally
              int* ok;
              int* prob;
};


void func(){
    Widget* pw = NULL;  // <<<<--------------- Issue in Here
    try{
        pw = new Widget;
    }
    catch(...){
               delete pw;
               std::cout<<"\nIn catch BLOCK\n";
               if(pw->ok == NULL){
                      std::cout<<"\n OK is NULL\n";
               }
               throw;
    }
    delete pw;
}

int main(){
    try{
          func();
    }
    catch(std::bad_alloc&){
                     std::cout<<"\nError allocating memory."<<std::endl;
    }

    std::cout<<std::endl;
    system("pause");
    return 0;
}

Now in function func(), i am seeing two different behavior depending on if i do not set pointer 'pw' to NULL and if i do set 'pw' pointer to NULL (like code above). I was under the impression that it is 'good' practise to first set pointer to NULL and then initialize it. But when i initilize it to NULL then the output just shows "In catch BLOCK" and later app crashes. but if i do not set pw pointer to NULL, then i see destructor of pw is called and following output is shown w/o any app crash.

DtoR of Widget is called

In catch BLOCK

OK is NULL

Error allocating memory.

Press any key to continue . . .

My question is why such difference in one case when we are not initializing 'pw' pointer to NULL and in other case we are setting it to NULL. Why destructor is called in one case and why it was not called in another.

Note: The intent of this code is to throw bad_alloc exeption.

Upvotes: 2

Views: 226

Answers (6)

pstrjds
pstrjds

Reputation: 17428

You are seeing the app crash when you set pw to NULL because of the line

if (pw->ok == NULL)

You are dereferencing NULL which is causing the crash. In the other case you are deleting garbage which will give you undefined behavior.

Also, you should not use a pointer after calling delete on it. That can cause all kinds of weird behavior.

To explain more of what is happening. Your Widget constructor is throwing the allocation exception. In this case most likely the allocation for ok completed, but the allocation for prob failed. Your constructor never finishes, leaking the memory that was allocated to the ok variable. If you want to ensure the memory is cleaned up, you should add a try catch in your constructor.

Widget() : ok(NULL), prob(NULL)
{
    try
    {
        ok = new int [1024];
        prob = new int [100*MAX];
    }
    catch(...)
    {
        std::cout << "\nCtor of Widget threw exception\n";
        delete [] ok;
        delete [] prob;
        throw;
    }
}

Upvotes: 0

rene
rene

Reputation: 166

you are going to intend the bad_alloc exeption.but you have more unhandled exceptions! It is not ok to delete pw first and then using the pointer of it! 

           delete pw;
           if(pw->ok == NULL)

Upvotes: 1

Mike Seymour
Mike Seymour

Reputation: 254431

In your catch block, you have:

if(pw->ok == NULL)

At this point, pw is NULL (or garbage, in the case that you didn't initialise it). pw-ok attempts to dereference it, giving undefined behaviour (a crash in this case).

If you didn't initialise it, then the delete pw will crash before printing the "catch" message; most likely, it will print the "Dtor" message before crashing, but there is no guarantee since you're firmly in the realm of undefined behaviour.

If you did initialise it, then delete pw is unnecessary but harmless; deleting a null pointer is defined to do nothing. So in that case you won't crash until you dereference it.

In any event, you have an unfixable memory leak - the first allocation ok = new int[1024] will have succeeded, but you have lost the only pointer to it. This is why you should always manage dynamic memory using smart pointers, containers, and other RAII techniques.

Upvotes: 4

Apprentice Queue
Apprentice Queue

Reputation: 2036

  1. Why would you call pw->ok after you delete pw? It's already gone.
  2. You constructor should have member initializes

Widget():ok(NULL), prob(NULL) {
...
}

because if Widget() fails, you don't know which member variable is initialized and which is not which can cause problem in your destructor.

  1. Since you allocated with int[], you need to delete[] not just delete in your destructor.

Upvotes: 0

f00l
f00l

Reputation: 59

It's good to initialize pw to NULL, but when deleting it, you should first check if pw is not null. Something like:

if (pw) delete pw;

Also, if pw is NULL, you can't reference its members.

Upvotes: -1

user405725
user405725

Reputation:

If you do not set pw to NULL then you will leave it uninitialized. Then, when new operator inside a "try" block throws an exception, it never returns, and you get into catch block. Since new never returned, pw will still be not initialized, and you will pass a random address to delete. That gives you an undefined behavior.

Upvotes: 6

Related Questions