Re: [PATCH v2 06/21] gdbstub: move GDBState to shared internals header

["Alex Bennée" <alex.bennee@linaro.org>]

Richard Henderson <richard.henderson@linaro.org> writes:

> On 2/21/23 00:24, Alex Bennée wrote:
>> Richard Henderson <richard.henderson@linaro.org> writes:
>> 
>>> On 1/5/23 08:43, Alex Bennée wrote:
>>>> We are about to split softmmu and user mode helpers into different
>>>> files. To facilitate this we will need to share access to the GDBState
>>>> between those files.
>>>> To keep building we have to temporarily define CONFIG_USER_ONLY just
>>>> before we include internals.h for the user-mode side of things. This
>>>> will get removed once the state is fully moved.
>>>
>>> You don't have to have this hack if you don't ...
>>>
>>>> +typedef struct GDBState {
>>>> +    bool init;       /* have we been initialised? */
>>>> +    CPUState *c_cpu; /* current CPU for step/continue ops */
>>>> +    CPUState *g_cpu; /* current CPU for other ops */
>>>> +    CPUState *query_cpu; /* for q{f|s}ThreadInfo */
>>>> +    enum RSState state; /* parsing state */
>>>> +    char line_buf[MAX_PACKET_LENGTH];
>>>> +    int line_buf_index;
>>>> +    int line_sum; /* running checksum */
>>>> +    int line_csum; /* checksum at the end of the packet */
>>>> +    GByteArray *last_packet;
>>>> +    int signal;
>>>> +#ifdef CONFIG_USER_ONLY
>>>> +    GDBUserState user;
>>>> +#else
>>>> +    GDBSystemState system;
>>>> +#endif
>>>
>>> ... nest these.  What's the point?
>> Well you end up having to ensure the chardev definitions are then
>> available in all files that include internals.h and I'm not sure that is
>> better:
>> --8<---------------cut here---------------start------------->8---
>> modified   gdbstub/internals.h
>> @@ -33,18 +33,16 @@ enum RSState {
>>   };
>>     /* Temporary home */
>> -#ifdef CONFIG_USER_ONLY
>>   typedef struct {
>>       int fd;
>>       char *socket_path;
>>       int running_state;
>>   } GDBUserState;
>> -#else
>> +
>>   typedef struct {
>>       CharBackend chr;
>>       Chardev *mon_chr;
>>   } GDBSystemState;
>> -#endif
>
> No, these typedefs and their data can be completely private to the two
> implementations.

Ahh right, so I've split that up now in:

  gdbstub: define separate user/system structures

and its made the subsequent patches much cleaner

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro