Re: [PATCH v2 3/5] common: console: introduce overflow and isempty calls

[Mattijs Korpershoek <mkorpershoek@baylibre.com>]

Hi Svyatoslav,

Thank you for your patch.

On mer., nov. 15, 2023 at 17:38, Svyatoslav Ryhel <clamor95@gmail.com> wrote:

> From: Ion Agorria <ion@agorria.com>
>
> Separate record overflow logic and add console_record_isempty
> as available calls don't serve to know output has been read
> fully with readline's.
>
> Signed-off-by: Ion Agorria <ion@agorria.com>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
>  common/console.c  | 15 ++++++++++++---
>  include/console.h | 14 ++++++++++++++
>  test/ut.c         |  9 ++++-----
>  3 files changed, 30 insertions(+), 8 deletions(-)
>
> diff --git a/common/console.c b/common/console.c
> index 98c3ee6ca6..8a869b137e 100644
> --- a/common/console.c
> +++ b/common/console.c
> @@ -818,6 +818,8 @@ int console_record_init(void)
>  	ret = membuff_new((struct membuff *)&gd->console_in,
>  			  CONFIG_CONSOLE_RECORD_IN_SIZE);
>  
> +	gd->flags |= GD_FLG_RECORD;
> +
>  	return ret;
>  }
>  
> @@ -836,11 +838,13 @@ int console_record_reset_enable(void)
>  	return 0;
>  }
>  
> -int console_record_readline(char *str, int maxlen)
> +bool console_record_overflow(void)
>  {
> -	if (gd->flags & GD_FLG_RECORD_OVF)
> -		return -ENOSPC;
> +	return gd->flags & GD_FLG_RECORD_OVF ? true : false;
> +}
>  
> +int console_record_readline(char *str, int maxlen)
> +{
>  	return membuff_readline((struct membuff *)&gd->console_out, str,
>  				maxlen, '\0');
>  }
> @@ -850,6 +854,11 @@ int console_record_avail(void)
>  	return membuff_avail((struct membuff *)&gd->console_out);
>  }
>  
> +bool console_record_isempty(void)
> +{
> +	return membuff_isempty((struct membuff *)&gd->console_out);
> +}
> +

We should also add a stub for console_record_isempty(), otherwise build
errors will creep in when when CONFIG_CONSOLE_RECORD is not set.

For example, on this series, we can see:

drivers/fastboot/fb_command.c: In function 'fastboot_multiresponse':
drivers/fastboot/fb_command.c:171:29: warning: implicit declaration of function 'console_record_isempty'; did you mean 'console_record_reset'? [-Wimplicit-function-declaration]
  171 |                         if (console_record_isempty()) {
      |                             ^~~~~~~~~~~~~~~~~~~~~~
      |                             console_record_reset


The following diff fixes it:

diff --git a/include/console.h b/include/console.h
index c053bc9ba82c..b5adae740650 100644
--- a/include/console.h
+++ b/include/console.h
@@ -145,6 +145,12 @@ static inline int console_in_puts(const char *str)
        return 0;
 }
 
+static inline bool console_record_isempty(void)
+{
+       /* Always empty */
+       return true;
+}
+

With that addressed, please add:

Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>

>  int console_in_puts(const char *str)
>  {
>  	return membuff_put((struct membuff *)&gd->console_in, str, strlen(str));
> diff --git a/include/console.h b/include/console.h
> index ceb733b5cb..c053bc9ba8 100644
> --- a/include/console.h
> +++ b/include/console.h
> @@ -64,6 +64,13 @@ void console_record_reset(void);
>   */
>  int console_record_reset_enable(void);
>  
> +/**
> + * console_record_overflow() - returns state of buffers overflow
> + *
> + * Return: true if the console buffer was overflowed
> + */
> +bool console_record_overflow(void);
> +
>  /**
>   * console_record_readline() - Read a line from the console output
>   *
> @@ -84,6 +91,13 @@ int console_record_readline(char *str, int maxlen);
>   */
>  int console_record_avail(void);
>  
> +/**
> + * console_record_isempty() - Returns if console output is empty
> + *
> + * Return: true if empty
> + */
> +bool console_record_isempty(void);
> +
>  /**
>   * console_in_puts() - Write a string to the console input buffer
>   *
> diff --git a/test/ut.c b/test/ut.c
> index 28da417686..d202644a15 100644
> --- a/test/ut.c
> +++ b/test/ut.c
> @@ -53,15 +53,14 @@ long ut_check_delta(ulong last)
>  
>  static int readline_check(struct unit_test_state *uts)
>  {
> -	int ret;
> -
> -	ret = console_record_readline(uts->actual_str, sizeof(uts->actual_str));
> -	if (ret == -ENOSPC) {
> +	if (console_record_overflow()) {
>  		ut_fail(uts, __FILE__, __LINE__, __func__,
>  			"Console record buffer too small - increase CONFIG_CONSOLE_RECORD_OUT_SIZE");
> -		return ret;
> +		return -ENOSPC;
>  	}
>  
> +	console_record_readline(uts->actual_str, sizeof(uts->actual_str));
> +
>  	return 0;
>  }
>  
> -- 
> 2.40.1