Re: [PATCH 1/4] serial: fix circular rx buffer edge case

[Rasmus Villemoes <ravi@prevas.dk>]

Simon Glass <sjg@chromium.org> writes:

> On Thu, 3 Oct 2024 at 08:10, Rasmus Villemoes <ravi@prevas.dk> wrote:
>>
>>  drivers/serial/serial-uclass.c | 10 ++++++----
>>  include/serial.h               |  4 ++--
>>  2 files changed, 8 insertions(+), 6 deletions(-)
>
> Reviewed-by: Simon Glass <sjg@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.

Also, which tests? I don't see membuff mentioned anywhere under test/,
but perhaps it's implicitly through the console recording testing?

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.

Rasmus