0

I have a complex class A:

class A{
...
};

and class B is include A's pointer and has a method to return the pointer.

class B{
A* GetA(){return pA;}
A* pA;
};

Then, when I use 'GetA' to get a A's pointer, it is easier to use the illegal pointer, like:

void func(A* p){
B b;
p = b.GetA();
}

when I use 'func', I just get an illegal pointer. The code above is just a sample. And I found it is much easy to make this mistake when I am in a multi-thread environment.

Is there any method to let me sure to avoid these mistakes?

Thanks a lot

aasa
  • 207
  • 1
  • 7
  • 3
    Consider shared_ptr instead of the raw pointers? – Jaywalker Aug 02 '12 at 16:41
  • 2
    This is a coding mistake that triggers undefined behavior; it has nothing to do with multithreading. Doesn't your compiler warn you about cases like that? – Sergey Kalinichenko Aug 02 '12 at 16:42
  • 4
    Consider initializing `pA` in `B`'s constructor? There is no way of creating an instance of `B` then without a valid `pA`. – Damon Aug 02 '12 at 16:42
  • 1
    possible duplicate of [What is a smart pointer and when should I use one?](http://stackoverflow.com/questions/106508/what-is-a-smart-pointer-and-when-should-i-use-one) – Mike Seymour Aug 02 '12 at 16:43
  • Initialise it with ctor. It will make it valid during a lifecycle of A object. – tim-oleksii Aug 02 '12 at 16:43
  • Don't use naked pointers. If you must use naked pointers, make a dedicated, single-responsibility SBRM wrapper class for it. – Kerrek SB Aug 02 '12 at 16:51
  • The first rule of avoiding dangling pointers is... avoid dangling pointers. – MrFox Aug 02 '12 at 18:18

2 Answers2

2

There is no general method in C++ to protect yourself from illegal pointer accesses, but there are idioms you can use to help protect yourself.

Keep your classes as representing concepts/objects and don't expose direct access to any pointers they might contain. Prefer for example descriptive methods, or for container-like classes begin/end style methods.

When managing pointers, prefer first to use a standard container such as vector. They're highly tuned and debugged and will prevent many problems since they already have all the right constructors, assignment, destructor, etc.

If a standard container isn't possible prefer a smart pointer such as unique_ptr, scoped_ptr, shared_ptr or weak_ptr depending on your pointer needs. By using an appropriate smart pointer, again you make it almost impossible to unintentionally make coding mistakes.

Even using all these guidelines you can always bypass any protection you've attempted to implement. If you find yourself using a lot of casts you should step back and re-examine your design.

In your case not only would I make pA not be accessible via accessor, I wouldn't make it a raw pointer either.

Mark B
  • 95,107
  • 10
  • 109
  • 188
0

If you initialize the pointer in the constructor to NULL:

class B {
    B() {
        pA = NULL;
    }
    A* GetA(){return pA;}
    A* pA;
}

Then you can check to see if the pointer is non-NULL before using it.

void func(A* p){
    B b;
    p = b.GetA();
    if(p != NULL) {
        //use p
    }
}

Also, it seems strange that you're passing in a pointer to a function, then assigning to it without first using it. p = b.GetA(); will not change anything outside of func. Perhaps you mean to do something like one of these?

//Caller passes reference to their pointer, caller's pointer gets set.
void func(A *&p) {
    B b;
    p = b.GetA();
}
//Caller passes pointer to their pointer, caller's pointer gets set.
void func(A **p){
    B b;
    if(p != NULL) {
        *p = b.GetA();
    }
}
Eric Finn
  • 8,629
  • 3
  • 33
  • 42
  • I think it is only useful in the method 'func', and when the func return, the 'b' will be destructed, and then the return value p will be a illegal pointer and it will not equal to 'NULL', (0xcccc meybe),and it is useless to compare it with 'NULL' – aasa Aug 03 '12 at 00:48
  • It depends on how a valid value of `pA` is assigned. If you dynamically allocate the memory, the pointer will still be valid after `b` goes out of scope. But why would the value of the pointer change? The value doesn't change unless a new value is assigned to it. – Eric Finn Aug 03 '12 at 01:19
  • Let's think b is a container, and pA is the element from b. when I picked out pA, it means I have a copy pointer pA' to the pA which stored in b, and then pA' has no relationship with b. When b is destoried, the pA in b will be free. but, because pA' is a copy of pA, the pA' still points to the memory which has been free by pA. So, when I use pA' again, it will be an error. – aasa Aug 03 '12 at 02:05
  • Ah. If you free pA in B's destructor, then yes, copies of pA will point to an invalid memory location. However, you need to explicitly free pA in B's destructor. – Eric Finn Aug 03 '12 at 03:12
  • Yeah. pA is pointer to a block of data, and I should share it to several thread. I could not free it outside the b. It should be managed within b. So I want to find a method that when I free pA inside B, it can prevent me use these illegal pointers ouside the b. – aasa Aug 03 '12 at 04:07
  • @aasa Okay then, Mark's answer is what you want. – Eric Finn Aug 03 '12 at 17:15