Re: [PATCH v3 16/21] gdbstub: expose api to find registers

[Akihiko Odaki]

On 2024/02/03 20:44, Alex Bennée wrote:
> Akihiko Odaki <akihiko.odaki@daynix.com> writes:
>
> > On 2024/01/22 23:56, Alex Bennée wrote:
> > > Expose an internal API to QEMU to return all the registers for a vCPU.
> > > The list containing the details required to called gdb_read_register().
> > > Based-on: <20231025093128.33116-15-akihiko.odaki@daynix.com>
> > > Cc: Akihiko Odaki <akihiko.odaki@daynix.com>
> > > Message-Id: <20240103173349.398526-38-alex.bennee@linaro.org>
> > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> > > ---
> > > v3
> > >     - rm unused api functions left over
> > > ---
> > >    include/exec/gdbstub.h | 28 ++++++++++++++++++++++++++++
> > >    gdbstub/gdbstub.c      | 27 ++++++++++++++++++++++++++-
> > >    2 files changed, 54 insertions(+), 1 deletion(-)
> > > diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
> > > index da9ddfe54c5..eb14b91139b 100644
> > > --- a/include/exec/gdbstub.h
> > > +++ b/include/exec/gdbstub.h
> > > @@ -111,6 +111,34 @@ void gdb_feature_builder_end(const GDBFeatureBuilder 
> > > *builder);
> > >     */
> > >    const GDBFeature *gdb_find_static_feature(const char *xmlname);
> > >    +/**
> > > + * gdb_read_register() - Read a register associated with a CPU.
> > > + * @cpu: The CPU associated with the register.
> > > + * @buf: The buffer that the read register will be appended to.
> > > + * @reg: The register's number returned by gdb_find_feature_register().
> > > + *
> > > + * Return: The number of read bytes.
> > > + */
> > > +int gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
> > > +
> > > +/**
> > > + * typedef GDBRegDesc - a register description from gdbstub
> > > + */
> > > +typedef struct {
> >
> > nit: Add struct name; docs/devel/style.rst says struct has a CamelCase
> > name *and* corresponding typedef, though this rule is apparently not
> > strictly enforced.
>
> I think the wording is a little ambiguous here, especially with the
> reference to typedefs.h where the anonymous structure typedefs are held.
> In this case we don't need the structname because there is no internal
> reference to itself.

The majority of structure typedefs have tag names though; grep "typedef 
struct".

That said, I'm fine even without a tag name. As I mentioned earlier, 
this convention in QEMU is not consistent, and I have no other reason to 
have a tag name.