Skip to main content
code-quote code
Source Link
Toby Speight
  • 88.4k
  • 14
  • 104
  • 327

The code overwrites object memory in push()push(). For anything besides trivial types this will be problematic (memory leaks, crashes).

The name RingBufferRingBuffer can be a bit misleading, maybe rename it to something like BlockingQueue BlockingQueue?

Instead of managing a raw T*T* buffer pointer, it'll likely be better if you used std::queuestd::queue.

This will not work if there are multiple concurrent consumer or provider threads (for example they may end up setting the same next_idxnext_idx multiple times and have a race condition on creating TT).

The args used when creating TT in push()push() are always passed by const reference, making it impossible to use T'sT's move-constructor ( Like, push(std::move(T()))push(std::move(T())) will still invoke the copy-constructor).

The code overwrites object memory in push(). For anything besides trivial types this will be problematic (memory leaks, crashes).

The name RingBuffer can be a bit misleading, maybe rename it to something like BlockingQueue ?

Instead of managing a raw T* buffer pointer, it'll likely be better if you used std::queue.

This will not work if there are multiple concurrent consumer or provider threads (for example they may end up setting the same next_idx multiple times and have a race condition on creating T).

The args used when creating T in push() are always passed by const reference, making it impossible to use T's move-constructor ( Like, push(std::move(T())) will still invoke the copy-constructor).

The code overwrites object memory in push(). For anything besides trivial types this will be problematic (memory leaks, crashes).

The name RingBuffer can be a bit misleading, maybe rename it to something like BlockingQueue?

Instead of managing a raw T* buffer pointer, it'll likely be better if you used std::queue.

This will not work if there are multiple concurrent consumer or provider threads (for example they may end up setting the same next_idx multiple times and have a race condition on creating T).

The args used when creating T in push() are always passed by const reference, making it impossible to use T's move-constructor ( Like, push(std::move(T())) will still invoke the copy-constructor).

Source Link

The code overwrites object memory in push(). For anything besides trivial types this will be problematic (memory leaks, crashes).

The name RingBuffer can be a bit misleading, maybe rename it to something like BlockingQueue ?

Instead of managing a raw T* buffer pointer, it'll likely be better if you used std::queue.

This will not work if there are multiple concurrent consumer or provider threads (for example they may end up setting the same next_idx multiple times and have a race condition on creating T).

The args used when creating T in push() are always passed by const reference, making it impossible to use T's move-constructor ( Like, push(std::move(T())) will still invoke the copy-constructor).