Re: [PATCH] cmdline: only set ask mode if vga= is present

[Jan Beulich <jbeulich@xxxxxxxx>]

On 10.07.2023 16:12, Roger Pau Monne wrote:
> Commit 9473d9a24182 set the ASK mode without checking if there was a
> `vga` option provided in the command line.  This breaks existing
> behavior, so exit early without changes if `vga` is not present in the
> command line.
> 
> Fixes: 9473d9a24182 ('cmdline: parse multiple instances of the vga option')
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Should have spotted this during review; effectively you're (almost) undoing
part of the earlier change, just that ...

> --- a/xen/arch/x86/boot/cmdline.c
> +++ b/xen/arch/x86/boot/cmdline.c
> @@ -279,9 +279,13 @@ static void vga_parse(const char *cmdline, 
> early_boot_opts_t *ebo)
>  {
>      const char *c = cmdline;
>  
> +    c = find_opt(c, "vga=", true);

... you use c instead of cmdline here (and I'm heavily tempted to actually
make this the initializer of c).

Jan

> +    if ( !c )
> +        return;
> +
>      ebo->boot_vid_mode = ASK_VGA;
>  
> -    while ( (c = find_opt(c, "vga=", true)) != NULL )
> +    do
>      {
>          unsigned int tmp, vesa_depth, vesa_height, vesa_width;
>  
> @@ -332,6 +336,7 @@ static void vga_parse(const char *cmdline, 
> early_boot_opts_t *ebo)
>          else if ( !strmaxcmp(c, "ask", delim_chars_comma) )
>              ebo->boot_vid_mode = ASK_VGA;
>      }
> +    while ( (c = find_opt(c, "vga=", true)) != NULL );
>  }
>  #endif
>