Re: [PATCH v3 01/13] monitor: Clean up HMP gdbserver error reporting

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 23/9/25 11:09, Markus Armbruster wrote:
> HMP command gdbserver used to emit two error messages for certain
> errors.  For instance, with -M none:
> 
>      (qemu) gdbserver
>      gdbstub: meaningless to attach gdb to a machine without any CPU.
>      Could not open gdbserver on device 'tcp::1234'

Orthogonal to this patch: interesting. This comes from commit
508b4ecc393 ("gdbstub.c: fix GDB connection segfault caused by empty
machines"). This feels wrong to me, as it is OK to use a jtag probe
to access memory or program a flash, even without any cpu online.

Yet another side-effect of use of the ill-first_cpu global variable.

Back to this patch description, probably not the best error example =)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> The first message is the specific error, and the second one a generic
> additional message that feels superfluous to me.
> 
> Commit c0e6b8b798b (system: propagate Error to gdbserver_start (and
> other device setups)) turned the first message into a warning:
> 
>      warning: gdbstub: meaningless to attach gdb to a machine without any CPU.
>      Could not open gdbserver on device 'tcp::1234'
> 
> This is arguably worse.
> 
> hmp_gdbserver() passes &error_warn to gdbserver_start(), so that
> failure gets reported as warning, and then additionally emits the
> generic error on failure.  This is a misuse of &error_warn.
> 
> Instead, receive the error in &err and report it, as usual.  With
> this, gdbserver reports just the error:
> 
>      gdbstub: meaningless to attach gdb to a machine without any CPU.
> 
> Cc: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   include/exec/gdbstub.h | 3 ---
>   monitor/hmp-cmds.c     | 7 ++++---
>   2 files changed, 4 insertions(+), 6 deletions(-)