[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 06/21] gdbstub: move GDBState to shared internals header
From: |
Alex Bennée |
Subject: |
Re: [PATCH v2 06/21] gdbstub: move GDBState to shared internals header |
Date: |
Tue, 21 Feb 2023 10:58:33 +0000 |
User-agent: |
mu4e 1.9.21; emacs 29.0.60 |
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