Hi, Alex
On Tue, Feb 3, 2026 at 5:51 AM Alex Bennée <[email protected]> wrote:
>
> We already set a default error reply which we can only overwrite if we
> successfully follow the chain of checks. Initialise the variables as
> NULL and use that to gate the construction of the filled out
> stop/reply packet.
>
> Signed-off-by: Alex Bennée <[email protected]>
Thanks for the patch: glad to see a goto gone.
> ---
> gdbstub/gdbstub.c | 30 +++++++++++++-----------------
> 1 file changed, 13 insertions(+), 17 deletions(-)
>
> diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
> index 1f8cd118924..d4db7ba30cc 100644
> --- a/gdbstub/gdbstub.c
> +++ b/gdbstub/gdbstub.c
> @@ -1413,36 +1413,32 @@ static void handle_v_cont(GArray *params, void
> *user_ctx)
>
> static void handle_v_attach(GArray *params, void *user_ctx)
> {
> - GDBProcess *process;
> - CPUState *cpu;
> + GDBProcess *process = NULL;
> + CPUState *cpu = NULL;
>
> + /* Default error reply */
> g_string_assign(gdbserver_state.str_buf, "E22");
> - if (!params->len) {
> - goto cleanup;
> - }
> -
> - process = gdb_get_process(gdb_get_cmd_param(params, 0)->val_ul);
> - if (!process) {
> - goto cleanup;
> + if (params->len) {
> + process = gdb_get_process(gdb_get_cmd_param(params, 0)->val_ul);
> }
>
> - cpu = gdb_get_first_cpu_in_process(process);
> - if (!cpu) {
> - goto cleanup;
> + if (process) {
> + cpu = gdb_get_first_cpu_in_process(process);
> }
>
> - process->attached = true;
> - gdbserver_state.g_cpu = cpu;
> - gdbserver_state.c_cpu = cpu;
> + if (cpu) {
> + process->attached = true;
> + gdbserver_state.g_cpu = cpu;
> + gdbserver_state.c_cpu = cpu;
>
> if (gdbserver_state.allow_stop_reply) {
> g_string_printf(gdbserver_state.str_buf, "T%02xthread:",
> GDB_SIGNAL_TRAP);
> gdb_append_thread_id(cpu, gdbserver_state.str_buf);
> g_string_append_c(gdbserver_state.str_buf, ';');
> gdbserver_state.allow_stop_reply = false;
> -cleanup:
> - gdb_put_strbuf();
> }
> +
> + gdb_put_strbuf();
> }
>
The `cpu` gated block doesn't have a closing brace here thereby breaking
build, but it does in the next patch. Maybe a `git add -p` gone awry?
Also, would it make sense to move the `cpu` and the to-be nested block
below it into the `process` gated block to avoid the `cpu` check
whenever `process` is NULL?
Yodel
> static void handle_v_kill(GArray *params, void *user_ctx)
> --
> 2.47.3
>
>
