UB Fix: Iterator Invalidation in for_each_entity

[Larry Watterson 486]

template <class _Func> void for_each_entity(_Func & func)

{
    itertype(m_set_entity) it = m_set_entity.begin();
    for ( ; it != m_set_entity.end(); ++it) {
        LPENTITY entity = *it;
        // <Factor> Sanity check
        if (entity->GetSectree() != this) {
            sys_err("<Factor> SECTREE-ENTITY relationship mismatch");
            m_set_entity.erase(it);
            continue;
        }

        func(entity);
    }

}

There is an Undefined Behavior (UB) in this code.  

The iterator it is being erased, and then a continue statement is executed. Calling erase(it) invalidates the iterator pointing to the erased element, but the for loop's increment expression (++it) then attempts to increment this already invalidated iterator.  

The traditional fix for this would be writing it = m_set_entity.erase(it) and refactoring the for loop into a while loop (since erase already returns the next valid iterator).

Anyway, looking at it from a modern perspective, here is a much cleaner solution utilizing C++20:

 

template <class _Func> void for_each_entity(_Func & func)

{
    std::erase_if(m_set_entity, [this](LPENTITY entity) {
        if (!entity || entity->GetSectree() != this) {

            sys_err("<Factor> SECTREE-ENTITY relationship mismatch");

            return true;

        }

        return false;

    });
    std::ranges::for_each(m_set_entity, std::ref(func));
}

Btw you can choice universal ref or const lvalue ref for func parameter, it's related with your use case. It's just example solution.

Edited August 14 by Larry Watterson