Re: [PATCH v2 0/8] Remove XML files from gdbserver

Philipp Rudo
2018-01-25T13:11:50Z

Hi Alan,

in general the series looks fine for me. Just some nits/suggestions.

Philipp

On Wed, 24 Jan 2018 09:26:10 +0000
Alan Hayward <Alan.Hayward@arm.com> wrote:

> Following review comments from Yao, this patch series is mostly a
> reordering of the code from the previous patch series.
>
> This set of patches removes the need for gdbserver to ship the xml files
> in the binary for those targets that use new style target descriptions.
>
> In exisiting code, gdbserver uses C code auto generated from xml files to
> create target descriptions. When sending an xml description to GDB, it
> creates an xml containing just the name of the original xml file.
> Upon receipt, GDB reads and parses a local copy of xml file.
>
> With this new patch, we add common code that allows gdbserver and gdb
> to turn a C target description structure into xml. To do this we must
> first ensure that when gdbserver parses a target description it retains
> enough information to turn it back into xml.
>
> The second patch commonises tdesc_reg, the third patch commonises
> tdesc_feature and the fifth patch commonises the tdesc type structures.
> This enables gdbserver to store all required information about a target
> description.
>
> The sixth patch adds the xml printer.
> The other patches are clean up patches.
>
> Patches have been tested on a make check on x86 targets=all build with
> target board unix native-gdbserver. Also tested aarch64. Built for power
> (becuase it does not use new target descriptions), but am unable to test.
> In addition, patch four adds new test cases to unit test.
>
> Alan.
>
>
> gdb/Makefile.in | 1 -
> gdb/arch/tdesc.c | 425 ------------------------
> gdb/arch/tdesc.h | 307 +-----------------
> gdb/features/aarch64-core.c | 2 +-
> gdb/features/aarch64-fpu.c | 2 +-
> gdb/features/i386/32bit-avx.c | 2 +-
> gdb/features/i386/32bit-avx512.c | 2 +-
> gdb/features/i386/32bit-core.c | 2 +-
> gdb/features/i386/32bit-linux.c | 2 +-
> gdb/features/i386/32bit-mpx.c | 2 +-
> gdb/features/i386/32bit-pkeys.c | 2 +-
> gdb/features/i386/32bit-sse.c | 2 +-
> gdb/features/i386/64bit-avx.c | 2 +-
> gdb/features/i386/64bit-avx512.c | 2 +-
> gdb/features/i386/64bit-core.c | 2 +-
> gdb/features/i386/64bit-linux.c | 2 +-
> gdb/features/i386/64bit-mpx.c | 2 +-
> gdb/features/i386/64bit-pkeys.c | 2 +-
> gdb/features/i386/64bit-segments.c | 2 +-
> gdb/features/i386/64bit-sse.c | 2 +-
> gdb/features/i386/x32-core.c | 2 +-
> gdb/features/tic6x-c6xp.c | 2 +-
> gdb/features/tic6x-core.c | 2 +-
> gdb/features/tic6x-gp.c | 2 +-
> gdb/gdbserver/Makefile.in | 3 -
> gdb/gdbserver/configure.srv | 36 +++
> gdb/gdbserver/tdesc.c | 229 +++++++------
> gdb/gdbserver/tdesc.h | 58 +++-
> gdb/regformats/regdat.sh | 5 +-
> gdb/target-descriptions.c | 924 +++++++++++++++++++++++++++++++++++++++--------------
> gdb/xml-tdesc.c | 9 -
> gdb/xml-tdesc.h | 5 -
> 32 files changed, 920 insertions(+), 1124 deletions(-)
>