Check valid pointer...

[ryu]

Hi GURUs,

I have a problem that will tickle GURUs brain

Imagine I have the following class:

class clsA
{
public:
   clsB *pObjB;

public:
   clsA()
   {
       pObjB = NULL;
   };
   ~clsA()
   {
      if(pObjB) delete pObjB;
   };
};

void main()
{
   clsA *pObjA1 = new clsA();
   clsA *pObjA2 = new clsA();

   pObjA1->pObjB = new clsB();
   pObjA2->pObjB = pObjA1->pObjB;

   delete pObjA1;   //the destructor will delete pObjB
   delete pObjA2;   //this will cause problem in the destructor since pObjB is not a valid pointer anymore (already been deleted previously by pObjA1 destructor)
}

As you can see, GURU, pObjB is been created once. Since both pObjA1 and pObjA2 point to the same pObjB, it will cause the memory exception because pObjA2 will try to delete pObjB that has been deleted by pObjA1.

My question is, how can I check whether pObjB is a valid pointer before I delete it in the destructor?

Thanks for any help from GURU(s) in advance...

Cheers

[ryu]

Ahh... got it... you could use AfxIsMemoryBlock() to check whether the memory block is valid or not.

Cheers

[Gnail]

Hi,

You may need to check try this out on Release Build.

From the msdn, it say AfxIsMemoryBlock is part of MFC Diagnostic Services..

These macros and functions are available for all classes derived from CObject in the Debug and Release versions of MFC. However, all except DEBUG_NEW and VERIFY do nothing in the Release version.

[developerid]

whenever u r going to release memory..I mean before calling delete, put a check, whether that pointer is NULL or not.If not than delete it and assign to that pointer NULL.

This approach should not create problem...

[Andreas Masur]

Well....In general you should reconsider your design in my eyes. Of course, you can get around this by applying 0 to the pointer after you have deleted it (no need to check before deletion though since deleting a 0-pointer is valid in C++).

However, this still leaves one problem with the code. As soon as the pointer is deleted, you get a dangling pointer in your 'pObjA2' instance. If you try to access this member 'pObjA2->pObjB' you will simply fail and an access violation is the most expected result.

Why do you need to have references to the same internal object from different instances of the class in the first place?

[Paul McKenzie]

Hi GURUs,

I have a problem that will tickle GURUs brain

It won't tickle mine -- I would scrap this whole design you've come up with and come up with something maintainable.

The problem with what you have is that it is unmaintainable in a real program without introducing all sorts of bugs. You may get away with it in a toy program, but this sort of memory management gymnastics just leads to bugs and long nights of figuring out why things don't work.

My question is, how can I check whether pObjB is a valid pointer before I delete it in the destructor?

In general in C++, you can't determine if a pointer is valid or not. Anyway, code that has "if this pointer is valid, delete it" is stating right there that the code is buggy. You should never need to write code to test for an invalid pointer.

Regards,

Paul McKenzie

[suchuhui80]

append a operator function in your class A to
prevent accessing private member directly (praticularly pointer)
this helps making your code safer.
do you agree ?

[souldog]

If you really need to hold references to an object in more than one class, then
you should be using a smart pointer. A reference counted pointer will work in
your case. You can download the source code for such a pointer from
www.boost.org. Shared pointer will work for you.

[ryu]

Hi SoulDog,

Thanks for your help

Now, that is a real solution. Thanks GURU.

The reason that I need to point to the same class is that this class hold about 100MB of data. I know most of the people will ask me to re-design this, but (trust me) my manager will kill me.

The design of this application has been around for 15 years and (I heard) it took 5 years to complete this application. I am just joining this company and I can't just go to upper management and tell them to change the design.

What I did before is to make a copy (originally, it points to the same object) of the class every time I need it. This method slow the process down to 1job per 45minutes (originally, 1job per 2seconds) and it is not acceptable.

That is why I said the souldog solution is ingenious workaround.

Thanks, GURU.

Cheers

By the way, thanks for the help of all the GURUs.