0

I'm new to UE4, and I encountered the general pattern of checking each pointer before using it, for example:

AActor *pActor = getOwner(); 
if(pActor){
   pActor->SOME_FUNCTION();
}

I'm aware to the fact that crashes are awful in terms of user experience, and that it is always advised to write code with maximum robustness, but are there well-known cases in which the pattern is really useful? (for example in theoretically safe settings, such as a pointer set to GetOwner()).

And if so, are there any common alternatives for this? (perhaps making use of smart pointers?)

EDIT: In my question I wanted to get an answer regarding UE4 specifically, since I encountered a source relating to UE4 only, which advised me to check pointers, although I wasn't sure about its necessity and in which pattern.

Eliran Abdoo
  • 611
  • 6
  • 17
  • 1
    what is getOwner? Is it expected to return the null pointer? What is happening after the branch? – SergeyA Jan 06 '17 at 19:29
  • @SergeyA GetOwner is a UE4 function, which is expected to return a pointer to the owning actor of a component. – Eliran Abdoo Jan 06 '17 at 19:34
  • Question is, are you expecting to have components without actors? What does it mean? – SergeyA Jan 06 '17 at 19:46
  • 1
    By the way, a comment by a staff on [this question](https://answers.unrealengine.com/questions/422693/using-getowner-crash-the-editor.html) suggest that `getOwner` can indeed return a null pointer. And there are no real alternatives to checking as you are receiving a raw pointer - smart pointers would not help here in any way. – UnholySheep Jan 06 '17 at 20:04
  • @SergeyA Indeed. For example a Pawn (which is the player's body, and also inherits from Actor), has a Physics Handle component for grabbing items. – Eliran Abdoo Jan 06 '17 at 20:34
  • Possible duplicate of [Should I always check member pointers for nullptr?](http://stackoverflow.com/questions/39715484/should-i-always-check-member-pointers-for-nullptr) – realvictorprm Jan 06 '17 at 20:54

2 Answers2

1

Since this is tagged C++, there is an obvious alternative for a pointer that must not be NULL: a reference. (see basic considerations or technical details for more information).

If you stay with pointers, you have - by definition - to deal with the possibility that each and everyone of them is NULL.

Update: A pretty silly example:

AActor & get_existing_owner() 
{
    AActor * pActor = getOwner(); 
    check(pActor);
    return *pActor;
}

Please note that this changes the behavior of your code in case getOwner actually returns nullptr some time (cf. UE Assertions). Then you could write somewhere else (without further checks):

get_existing_owner().some_function();

Obviously above get_existing_... checking method could be generealized / reused for other pointers.

Community
  • 1
  • 1
m8mble
  • 1,513
  • 1
  • 22
  • 30
  • You mean that I should assign each pointer I try to get from a function, to a reference ? – Eliran Abdoo Jan 06 '17 at 19:37
  • @GoldenSpecOps: Nope, that won't help a bit: You'll get undefined behavior if you convert a `nullptr` to a reference. The conversion will likely work fine (it's a noop as far as the machine is concerned), but some time later, you'll get the same crash as with the pointer. If you want to get safety out of using references, you must check the pointer *before* converting it to a reference. – cmaster - reinstate monica Jan 06 '17 at 19:40
  • No. You can't do much about the return value (eg. pointer) of some API-function. But you can use references in your code. I'll update with an example... – m8mble Jan 06 '17 at 19:40
  • @cmaster so basically using references doesn't contribute anything since I'll still have to check each pointer – Eliran Abdoo Jan 06 '17 at 19:42
  • Dropping the checks will abort the program whenever the assumption is ever not met. This changes the code! – m8mble Jan 06 '17 at 19:46
  • Please at least use `nullptr` instead of the "old" macro `NULL`. The Unreal Engine Coding Standard [strongly suggests it](https://docs.unrealengine.com/latest/INT/Programming/Development/CodingStandard/#nullptr) – UnholySheep Jan 06 '17 at 19:49
  • Also by default Unreal Engine comes with disabled exception handling (see e.g.: [this question](https://answers.unrealengine.com/questions/264921/cannot-use-try-with-exceptions-disabled.html)) - so your *"silly example"* won't even work too well – UnholySheep Jan 06 '17 at 19:51
  • 1
    @GoldenSpecOps: Yes, and no: Yes, you still have to do the checking when you convert a pointer to a reference. However, using references moves the problem: Now it's up to the code converting the pointer to the reference to check the pointer, not to each and every bit of code using the reference. So, using references may help reducing the problem. It's basically like this: When some code produces a reference, it declares by using a reference, that it has already done the `nullptr` check, and that other code does not need to repeat it. – cmaster - reinstate monica Jan 06 '17 at 19:53
  • @cmaster I think you solved two of my problems at the same time. After getting the general sense that there's no running away from validating every pointer, I ran into a problem where I would use a pointer several times withing a code segment, and I thought I'd have to check it every single time. So basically checking it once and dereferencing it into a ref would significantly reduce the work (in case I understood you correctly) – Eliran Abdoo Jan 06 '17 at 20:29
  • 1
    @GoldenSpecOps if you already perform the check it doesn't make much difference whether you store it in a reference or pointer (except for a minor syntax difference). The usual pattern is to either wrap all access to a pointer in a single `if(ptr)` or have one check right after fetching the pointer and returning/asserting if it is null. – UnholySheep Jan 06 '17 at 20:36
  • @UnholySheep Oh, so I shouldn't go for the "paranoid" pattern, which means that I assume a pointer can get corrupted between any two lines of code (and hence check before every use)? – Eliran Abdoo Jan 07 '17 at 09:14
  • 1
    @GoldenSpecOps no, as long as it's within the same frame a pointed to object should never become deallocated or "corrupted" - if it does then you would have some massive threading issues (and you wouldn't be able to check for it particularly well either - the pointer you store would not contain the"updated" value - it would just point to an invalid location in memory). Note that you should not store pointers to objects that you are not the owner of for more than one frame - that could cause issues (instead request the pointer when you need it/once per frame) – UnholySheep Jan 07 '17 at 14:01
0

If you are not absolutely guaranteed to receive a valid pointer from a function that returns a pointer, you need to check that pointer. In the case of libraries that are black boxes (you don't own the code), you can perhaps take the word of the library author that the pointer will always be valid, but based on a comment above, that does not appear to be the case here.

Interestingly enough, in the C++ Core Guidelines: http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#S-gsl

There is a template in the Guidelines Support Library called not_null<T> which can be used to encapsulate a pointer to provide the guarantee that the pointer is never NULL.

Mikel F
  • 3,567
  • 1
  • 21
  • 33