On Wed, Mar 05 2025, Simon Glass <s...@chromium.org> wrote: > Hi Rasmus, > > On Tue, 4 Mar 2025 at 11:55, Rasmus Villemoes <r...@prevas.dk> wrote: >> >> On Tue, Mar 04 2025, Simon Glass <s...@chromium.org> wrote: >> >> > Hi Rasmus, >> > >> > On Fri, 18 Oct 2024 at 08:55, Simon Glass <s...@chromium.org> wrote: >> >> >> >> Hi Rasmus, >> >> >> >> On Fri, 18 Oct 2024 at 01:05, Rasmus Villemoes <r...@prevas.dk> wrote: >> >> > >> >> > >> >> > If you want to do the churn of renaming anyway, I suggest doing it by >> >> > adding an implementation using the proper scheme under the new name, >> >> > switch users over, and dropping the old. IMO, this series as-is brings >> >> > no value (except for the tests, of course). >> >> >> >> OK. Do you think this series gets us closer to that, or further away? >> > >> > I didn't get a response to this (which is not a problem, I miss things >> > all the time). Anyway I don't like the power-of-two restriction and >> > you can see my other responses above. I've applied this to my tree as >> > I want to have the tests in place. >> >> I stand by my earlier comments that this is the wrong way to implement a >> circular buffer. I hope Tom doesn't pull this. > > OK. Are you saying that you think it should only support power-of-two > sizes,
Yes, because that's the natural way to implement such a simple data structure on real hardware, and I don't agree that it limits its usefulness in any way. See how the linux kernel implements unix pipes, and their kfifo helper. > or something else? What specifically do you want? I don't "want" anything in particular. I'm merely voicing my opinion that I consider this approach to implementing a circular buffer inferior. > This series: > - adds tests Yes, always a good thing. > - renames to membuf While we can agree that is better than with two f's, I don't see it as a huge virtue. > - shows how we could switch to using an empty/full flag instead of > leaving an empty slot, so we can see the code-size image As I've said previously, I believe that actually makes the whole thing even worse. Not using the natural wrapping/masking is already error-prone enough, but needing to have two parallel implementations living in the same source files with #ifdeffery; please, no. Do one or the other, unconditionally. > - does all this without requiring the size to be a power of two (which > limits its usefulness IMO) On that, we'll just have to agree to disagree. Rasmus PS: Something like https://gist.github.com/Villemoes/332ac7a5dfc983c58ad40773c7bc6385 is what I consider a simple and readable implementation (it may be buggy, of course). Frankly, the triple-pointer use in membuff.c frightens me, and that the implementation of something as simple as membuff_avail() involves a struct copy and calling membuff_getraw() in a loop makes it really hard to reason about and verify.
