Improve DynamicBVH code to make it clearer how the stack/heap works

[alesliehughes]

...
if (aux_stack.is_empty())
memcpy(aux_stack.ptr(), stack, ALLOCA_STACK_SIZE * sizeof(const Node *));
...
stack = aux_stack.ptr();

There might be a possibility that "aux_stack.ptr()" and "stack" will overlap. Using memmove will ensure in this case it's handled correctly.

[RandomShaper]

Looks to me like once stack = aux_stack.ptr(); happens the first time, the positive branch of the if won't be entered again. In other words, the positive branch is where the code switches from the "stack stack" to the heap stack, and from that point onwards the code won't copy again. (Resizing the LocalVector will implicitly copy to a new memory block if needed, which by the way is the reason the stack pointer has to be reassigned.)

If I'm right, a change that would communicate all that more clrearly would be to keep the original stack pointer as alloca_stack, then assign stack to it and replace the memcpy with the following:

memcpy(aux_stack.ptr(), alloca_stack, ...);
alloca_stack = nullptr;

[lawnjelly]

There might be a possibility that "aux_stack.ptr()" and "stack" will overlap. Using memmove will ensure in this case it's handled correctly.

Agree with @RandomShaper you seem to be misunderstanding how the code works. The alloca stack the OS stack, and the aux stack is actually the heap.

[alesliehughes]

There might be a possibility that "aux_stack.ptr()" and "stack" will overlap. Using memmove will ensure in this case it's handled correctly.

Agree with @RandomShaper you seem to be misunderstanding how the code works. The alloca stack the OS stack, and the aux stack is actually the heap.

This is true. I'm still new to the godot code base. I'll adjust the patch as suggested.

I'm just trying to fix Coverity issues that make sense in the short term.

[RandomShaper]

This is true. I'm still new to the godot code base. I'll adjust the patch as suggested.

I'm just trying to fix Coverity issues that make sense in the short term.

No worries. Besides, I'd advise you to get more familiar with the codebase until the point where you start feeling comfortable catching and fixing issues yourself. Automated tools only get you so far (for now!) and can even hinder your self-training process.

[alesliehughes]

Updated and tested to the best I can.

Advise on downloadable projects that would make sure I didn't break anything?

[RandomShaper]

LGTM. I think this makes the intent of the code more explicit. Aside, I'm pretty sure the compiler will be good at removing the superfluous (to the machine) stuff.

[lawnjelly]

Looks fine as far as I can see for functionality.

Whether it improves readability I can't say, but then I'm quite used to this pattern (stack then heap) so am not an unbiased observer.

[YuriSizov]

Thanks!