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

[Simon Glass]

Hi Rasmus,

On Wed, 9 Oct 2024 at 05:03, Rasmus Villemoes <ravi at prevas.dk> wrote:
>
> Simon Glass <sjg at chromium.org> writes:
>
> > On Thu, 3 Oct 2024 at 08:10, Rasmus Villemoes <ravi at 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 at 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 <sjg at chromium.org>

Regards,
SImon