ETLCPP / etl

Embedded Template Library
https://www.etlcpp.com
MIT License
2.25k stars 392 forks source link

Bug in queue pop (documentation), can break queue.empty() #948

Closed Brolaf-dev closed 2 months ago

Brolaf-dev commented 2 months ago
 //*************************************************************************
  /// Removes the oldest value from the back of the queue.
  /// Does nothing if the queue is already empty.
  /// If asserts or exceptions are enabled, throws an etl::queue_empty if the queue is empty.
  //*************************************************************************
        void pop()
        {
    #if defined(ETL_CHECK_PUSH_POP)
          ETL_ASSERT(!empty(), ETL_ERROR(queue_empty));
    #endif
          p_buffer[out].~T();
          del_out();
        }    

void del_out()
{
if (++out == CAPACITY) ETL_UNLIKELY
{
out = 0;
}
--current_size;
ETL_DECREMENT_DEBUG_COUNT
}

bool empty() const
{
return current_size == 0;
}

Either the function or the documentation here is incorrect, when calling pop on an empty queue it does not do nothing. It does lower the current_size by 1 resulting in current_size -1. This in turn causes empty to return false which is incorrect.

jwellbelove commented 2 months ago

I'm fixing queue & stack, but this issue with the use of ETL_ASSERT is more widespread and needs a more thorough library wide fix.