Re: [PATCH 2/2] semihosting/config: Merge --semihosting-config option groups

[Luc Michel <luc@lmichel.fr>]

On 20:00 Thu 26 May     , Peter Maydell wrote:
> Currently we mishandle the --semihosting-config option if the
> user specifies it on the command line more than once. For
> example with:
>  --semihosting-config target=gdb --semihosting-config arg=foo,arg=bar
> 
> the function qemu_semihosting_config_options() is called twice, once
> for each argument.  But that function expects to be called only once,
> and it always unconditionally sets the semihosting.enabled,
> semihost_chardev and semihosting.target variables.  This means that
> if any of those options were set anywhere except the last
> --semihosting-config option on the command line, those settings are
> ignored.  In the example above, 'target=gdb' in the first option is
> overridden by an implied default 'target=auto' in the second.
> 
> The QemuOptsList machinery has a flag for handling this kind of
> "option group is setting global state": by setting
>  .merge_lists = true;
> we make the machinery merge all the --semihosting-config arguments
> the user passes into a single set of options and call our
> qemu_semihosting_config_options() just once.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Luc Michel <luc@lmichel.fr>

> ---
>  semihosting/config.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/semihosting/config.c b/semihosting/config.c
> index 50d82108e6e..3afacf54ab2 100644
> --- a/semihosting/config.c
> +++ b/semihosting/config.c
> @@ -27,6 +27,7 @@
>  
>  QemuOptsList qemu_semihosting_config_opts = {
>      .name = "semihosting-config",
> +    .merge_lists = true,
>      .implied_opt_name = "enable",
>      .head = QTAILQ_HEAD_INITIALIZER(qemu_semihosting_config_opts.head),
>      .desc = {
> -- 
> 2.25.1
> 
> 

--