Hi Rasmus,
On Wed, 9 Oct 2024 at 05:03, Rasmus Villemoes ravi@prevas.dk wrote:
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.
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@chromium.org
Regards, SImon