Hi Rasmus, On Wed, 9 Oct 2024 at 05:03, Rasmus Villemoes <r...@prevas.dk> wrote: > > Simon Glass <s...@chromium.org> writes: > > > On Thu, 3 Oct 2024 at 08:10, Rasmus Villemoes <r...@prevas.dk> wrote: > >> > >> drivers/serial/serial-uclass.c | 10 ++++++---- > >> include/serial.h | 4 ++-- > >> 2 files changed, 8 insertions(+), 6 deletions(-) > > > > Reviewed-by: Simon Glass <s...@chromium.org> > > > > Perhaps we should use membuff, like in other cases, since it has some tests? > > I didn't know about that till just now. But no, it seems to suffer from > the same basic defect of losing one slot, and while it may handle it > correctly, I don't like to introduce more uses of that model, and > open-coding the single putc/getc that we need here is really not that > hard.
I will send a series which adds the 'full' flag in as an option. When I originally sent membuff I didn't include it. > > Also, which tests? I don't see membuff mentioned anywhere under test/, > but perhaps it's implicitly through the console recording testing? Ooop, no tests. Will include with the series. > > I just had a quick peek in the implementation, and membuff_makecontig() > seems to be buggy: the second memcpy() must also be a memmove() (suppose > size==32, head==30, tail==10; then the memcpy() does a 10-byte > overlap...). It has no users and never has had, so it doesn't matter > too much. Nor does it have any tests, even with my series, unfortunately. Let's see if anyone finds a use for it. Or if you are keen you could add some. Anyway: Reviewed-by: Simon Glass <s...@chromium.org> Regards, SImon
