Re: [PATCH v1 04/10] gdbstub: split GDBConnection from main structure

[Richard Henderson]

On 12/16/22 03:22, Alex Bennée wrote:
> In preparation for moving user/softmmu specific bits from the main
> gdbstub file we need to separate the connection details to what will
> eventually become an anonymous pointer.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   gdbstub/gdbstub.c | 99 +++++++++++++++++++++++++++--------------------
>   1 file changed, 58 insertions(+), 41 deletions(-)
>
> diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
> index be88ca0d71..14ce911bf2 100644
> --- a/gdbstub/gdbstub.c
> +++ b/gdbstub/gdbstub.c
> @@ -342,6 +342,20 @@ enum RSState {
>       RS_CHKSUM1,
>       RS_CHKSUM2,
>   };
> +
> +#ifdef CONFIG_USER_ONLY
> +typedef struct {
> +    int fd;
> +    char *socket_path;
> +    int running_state;
> +} GDBConnection;
> +#else
> +typedef struct {
> +    CharBackend chr;
> +    Chardev *mon_chr;
> +} GDBConnection;
> +#endif

You added the struct tag in the next patch.  Move it back if you need it, 
otherwise drop it.

> +    GDBConnection *connection;
>       bool multiprocess;
>       GDBProcess *processes;
>       int process_num;
> @@ -392,6 +399,12 @@ static void init_gdbserver_state(void)
>       gdbserver_state.supported_sstep_flags = 
> accel_supported_gdbstub_sstep_flags();
>       gdbserver_state.sstep_flags = SSTEP_ENABLE | SSTEP_NOIRQ | SSTEP_NOTIMER;
>       gdbserver_state.sstep_flags &= gdbserver_state.supported_sstep_flags;
> +
> +    /*
> +     * The following is differs depending on USER/SOFTMMU, we just
> +     * hid it in the typedef.
> +     */
> +    gdbserver_state.connection = g_new(GDBConnection, 1);

Does this structure really need this pointer?

Given that GDBState gdbserver_state is a global singleton, I would imagine that 
GDBConnection gdbserver_{sys,user}_state could also be global singletons, static within 
the two respective implementations.


r~