[PATCH 3/4] console: remove #ifdef CONFIG_CONSOLE_RECORD

[Simon Glass]

Hi Patrick,

On Thu, 3 Dec 2020 at 02:20, Patrick Delaunay <patrick.delaunay at st.com> wrote:
>
> Add helper functions to access to gd->console_out and gd->console_in
> with membuff API and replace the #ifdef CONFIG_CONSOLE_RECORD test
> by if (IS_ENABLED(CONFIG_CONSOLE_RECORD)) to respect the U-Boot
> coding rule.
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com>
> # Conflicts:
> #       common/console.c
> ---
>
>  common/console.c | 86 +++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 66 insertions(+), 20 deletions(-)

Reviewed-by: Simon Glass <sjg at chromium.org>

But see below

>
> diff --git a/common/console.c b/common/console.c
> index 9a63eaa664..0b864444bb 100644
> --- a/common/console.c
> +++ b/common/console.c
> @@ -88,6 +88,56 @@ static int on_silent(const char *name, const char *value, enum env_op op,
>  U_BOOT_ENV_CALLBACK(silent, on_silent);
>  #endif
>
> +#ifdef CONFIG_CONSOLE_RECORD
> +/* helper function: access to gd->console_out and gd->console_in */
> +static void console_record_putc(const char c)
> +{
> +       if  (gd->console_out.start)
> +               membuff_putbyte((struct membuff *)&gd->console_out, c);
> +}
> +
> +static void console_record_puts(const char *s)
> +{
> +       if  (gd->console_out.start)
> +               membuff_put((struct membuff *)&gd->console_out, s, strlen(s));
> +}
> +
> +static int console_record_getc(void)
> +{
> +       if (!gd->console_in.start)
> +               return -1;
> +
> +       return membuff_getbyte((struct membuff *)&gd->console_in);
> +}
> +
> +static int console_record_tstc(void)
> +{
> +       if (gd->console_in.start) {
> +               if (membuff_peekbyte((struct membuff *)&gd->console_in) != -1)
> +                       return 1;
> +       }
> +       return 0;
> +}
> +#else
> +static void console_record_putc(char c)
> +{
> +}
> +
> +static void console_record_puts(const char *s)
> +{
> +}
> +
> +static int console_record_getc(void)
> +{
> +       return -1;
> +}
> +
> +static int console_record_tstc(void)
> +{
> +       return 0;
> +}
> +#endif
> +
>  #if CONFIG_IS_ENABLED(SYS_CONSOLE_IS_IN_ENV)
>  /*
>   * if overwrite_console returns 1, the stdin, stderr and stdout
> @@ -420,15 +470,13 @@ int getchar(void)
>         if (!gd->have_console)
>                 return 0;
>
> -#ifdef CONFIG_CONSOLE_RECORD
> -       if (gd->console_in.start) {
> -               int ch;
> +       if (IS_ENABLED(CONFIG_CONSOLE_RECORD)) {
> +               int ch = console_record_getc();
>
> -               ch = membuff_getbyte((struct membuff *)&gd->console_in);
>                 if (ch != -1)
> -                       return 1;
> +                       return ch;
>         }
> -#endif
> +
>         if (gd->flags & GD_FLG_DEVINIT) {
>                 /* Get from the standard input */
>                 return fgetc(stdin);
> @@ -445,12 +493,10 @@ int tstc(void)
>
>         if (!gd->have_console)
>                 return 0;
> -#ifdef CONFIG_CONSOLE_RECORD
> -       if (gd->console_in.start) {
> -               if (membuff_peekbyte((struct membuff *)&gd->console_in) != -1)
> -                       return 1;
> -       }
> -#endif
> +
> +       if (IS_ENABLED(CONFIG_CONSOLE_RECORD) && console_record_tstc())
> +               return 1;
> +
>         if (gd->flags & GD_FLG_DEVINIT) {
>                 /* Test the standard input */
>                 return ftstc(stdin);
> @@ -521,10 +567,10 @@ void putc(const char c)
>  {
>         if (!gd)
>                 return;
> -#ifdef CONFIG_CONSOLE_RECORD
> -       if ((gd->flags & GD_FLG_RECORD) && gd->console_out.start)
> -               membuff_putbyte((struct membuff *)&gd->console_out, c);
> -#endif
> +
> +       if (IS_ENABLED(CONFIG_CONSOLE_RECORD) && (gd->flags & GD_FLG_RECORD))
> +               console_record_putc(c);

Given your functions above I wonder why you need the IS_ENABLED()
here? Maybe even move the gd-.flags check into those functions?

> +
>  #ifdef CONFIG_SANDBOX
>         /* sandbox can send characters to stdout before it has a console */
>         if (!(gd->flags & GD_FLG_SERIAL_READY)) {
> @@ -564,10 +610,10 @@ void puts(const char *s)
>  {
>         if (!gd)
>                 return;
> -#ifdef CONFIG_CONSOLE_RECORD
> -       if ((gd->flags & GD_FLG_RECORD) && gd->console_out.start)
> -               membuff_put((struct membuff *)&gd->console_out, s, strlen(s));
> -#endif
> +
> +       if (IS_ENABLED(CONFIG_CONSOLE_RECORD) && (gd->flags & GD_FLG_RECORD))
> +               console_record_puts(s);
> +
>  #ifdef CONFIG_SANDBOX
>         /* sandbox can send characters to stdout before it has a console */
>         if (!(gd->flags & GD_FLG_SERIAL_READY)) {
> --
> 2.17.1
>

Regards,
Simon