CharlesFrasch/cppcon2023

Could you please explain more details of fifo4b?

Closed this issue · 7 comments

pffang commented

I didn't see the explanation of fifo4b in your video of CppCon 2023.
This one looks not a correct implementation.
Could you please add more details?

CharlesFrasch commented

Would you please describe what about the implementation you feel is not correct.
R/cff

pffang commented

If you add fifo4 into unit tests, you will see push() trigger heap-buffer-overflow.
Because pushCursor tries to access the index out of capacity.

[ RUN      ] FifoTest/0.popFullFifo
capacity: 4
pushCursor: 0
nextPushCursor: 1
pushCursor: 1
nextPushCursor: 2
pushCursor: 2
nextPushCursor: 3
pushCursor: 3
nextPushCursor: 4
popCursor: 0
nextPopCursor: 1
pushCursor: **4**
=================================================================
unitTests.cpp:22: Failure
Failed
Encountered an address sanitizer error
CharlesFrasch commented
  • ❤️1
CharlesFrasch commented
  • 👍1
pffang commented

Hi @CharlesFrasch,

Sorry to trouble you again.
There's no address sanitizer error now, but I still have a doubt on fifo4b.

In unit tests, we initialize the fifo4b with capacity = 4, and fifo4b::capacity() also returns 4.
So the index of the cursor should be [0-3], but in fact, we can see both pushCursor and popCursor access index 4. We can save 5 elements in the fifo4b.

Why design like this?

CharlesFrasch commented
pffang commented

Understand, thanks