[PATCH v4 4/5] lib: membuff: fix readline not returning line in case of overflow

[Simon Glass]

Hi Svyatoslav,

On Thu, 23 Nov 2023 at 04:02, Svyatoslav Ryhel <clamor95 at gmail.com> wrote:
>
> From: Ion Agorria <ion at agorria.com>
>
> If a line overflows readline's maxlen it won't advance the membuffer
> and will return 0 as read amount which isn't even documented.
> Fix by removing this behavior alltogether.

spelling

But then how does one know the whole line has been read?

I don't think this is a good idea. We need to return some sort of
error condition.

I suggest either:

1. Add a new function which reads a line up to maxlen and discards
anything after that
2. returning -1 in the case that the line was too long, then having
the caller call a new membuff_skipline() function to skip it (which
supports any length)


>
> Signed-off-by: Ion Agorria <ion at agorria.com>
> Signed-off-by: Svyatoslav Ryhel <clamor95 at gmail.com>
> Reviewed-by: Mattijs Korpershoek <mkorpershoek at baylibre.com>
> ---
>  lib/membuff.c | 10 ----------
>  1 file changed, 10 deletions(-)
>
> diff --git a/lib/membuff.c b/lib/membuff.c
> index 36dc43a523..f582193dcd 100644
> --- a/lib/membuff.c
> +++ b/lib/membuff.c
> @@ -292,15 +292,12 @@ int membuff_readline(struct membuff *mb, char *str, int maxlen, int minch)
>  {
>         int len;  /* number of bytes read (!= string length) */
>         char *s, *end;
> -       bool ok = false;
> -       char *orig = str;
>
>         end = mb->head >= mb->tail ? mb->head : mb->end;
>         for (len = 0, s = mb->tail; s < end && len < maxlen - 1; str++) {
>                 *str = *s++;
>                 len++;
>                 if (*str == '\n' || *str < minch) {
> -                       ok = true;
>                         break;
>                 }
>                 if (s == end && mb->tail > mb->head) {
> @@ -309,13 +306,6 @@ int membuff_readline(struct membuff *mb, char *str, int maxlen, int minch)
>                 }
>         }
>
> -       /* couldn't get the whole string */
> -       if (!ok) {
> -               if (maxlen)
> -                       *orig = '\0';
> -               return 0;
> -       }
> -
>         /* terminate the string, update the membuff and return success */
>         *str = '\0';
>         mb->tail = s == mb->end ? mb->start : s;
> --
> 2.40.1
>