Libc ringbuff implementation

29jm / SnowflakeOS

"It is very special"

https://jmnl.xyz

MIT License

316 stars 18 forks source link

Libc ringbuff implementation #23

Closed xadaemon closed 3 years ago

xadaemon commented 3 years ago

I'm opening this PR with the libc ring-buffer mentioned in #15, I intend to submit a later pr fixing #15 itself but for now to allow usage elsewhere I decided to open this PR now.

I did some heavy commenting on the code in the hopes that it can sort of explain itself.

29jm commented 3 years ago

Just one last thing: unread_data is unbounded. Writing 100 bytes in a 10 bytes ringbuffer will tell you that there's -90 bytes of data available. Everything else looks great !

xadaemon commented 3 years ago

Just one last thing: unread_data is unbounded. Writing 100 bytes in a 10 bytes ringbuffer will tell you that there's -90 bytes of data available. Everything else looks great !

should be bounded now

29jm commented 3 years ago

Would you mind cherry-picking the last commit from the pr-ringbuffer branch, or making a commit with similar changes? Ring buffers are hard to get right, I tested thoroughly and found a few more issues:

r_pos wasn't being updated when data was overwritten; it should always point to the oldest data
unread_data was indeed bounded, but could now become smaller on writes ^^

I did away with w_pos, updating it independently from r_pos made things harder, since it's in fact entirely dependent on r_pos and unread_data.

Merging right after that!

xadaemon commented 3 years ago

I might take a little longer on my end today but should be done before long, thanks ^^

-------- Original Message -------- On 24 Dec 2020, 09:00, Johan Manuel wrote:

Would you mind cherry-picking the last commit from the pr-ringbuffer branch, or making a commit with similar changes? Ring buffers are hard to get right, I tested thoroughly and found a few more issues:

r_pos wasn't being updated when data was overwritten; it should always point to the oldest data

unread_data was indeed bounded, but could now become smaller on writes ^^

I did away with w_pos, updating it independently from r_pos made things harder, since it's in fact entirely dependent on r_pos and unread_data.

Merging right after that!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

xadaemon commented 3 years ago

small update, had a few personal issues, will get it cherry picked tomorrow

xadaemon commented 3 years ago

@29jm I did the cherry-pick as requested ^^

29jm commented 3 years ago

Thanks a lot!