[RF] fix memleak in rooabscollection::find

[ferdymercury]

This Pull request:

Changes or fixes:

Fixes #18322

Checklist:

• tested changes locally
• updated the docs (if necessary)

[github-actions]

Test Results

    18 files      18 suites   4d 16h 27m 49s ⏱️
 2 722 tests  2 722 ✅ 0 💤 0 ❌
47 305 runs  47 305 ✅ 0 💤 0 ❌

Results for commit c4cace9.

[dpiparo]

Can we merge?

[guitargeek]

What is _hashAssistedFind is a nullptr? Then this code branch will activate, and calling delete on a nullptr is not alllowed. Also, it would be better to set the pointer to nullptr after the delete, so there won't be a problem with dereferencing garbage pointers later

[ferdymercury]

What is _hashAssistedFind is a nullptr? Then this code branch will activate, and calling delete on a nullptr is not alllowed. Also, it would be better to set the pointer to nullptr after the delete, so there won't be a problem with dereferencing garbage pointers later

Delete on nullptr is okish, right?
According to cppreference: If ptr is a null pointer value, no destructors are called, and the deallocation function may or may not be called (it's unspecified), but the default deallocation functions are guaranteed to do nothing when passed a null pointer.

No need to set it to null after delete, right?
New is being called just below. In line 933.

Anyway I will change the first part as suggested, does not harm either.

[silverweed]

Indeed, calling delete on a nullptr is perfectly valid and harmless, so the check is superfluous.
I'll ask here as well if we should take the occasion to change that variable to a unique_ptr though...

[guitargeek]

Hi thanks @silverweed, I didn't know that! Sorry Ferdy for the confusion.

Unfortunately we can't migrate to std::unique_ptr until this PR is merged:

• TProtoClass: do not complain innards of transients. #16202

[guitargeek]

Thanks you very much for this PR! Sorry for making meaningless comments earlier, I really didn't know calling delete on a nullptr is okay :)