This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v2 6/8] Create xml from target descriptions
- From: Alan Hayward <Alan dot Hayward at arm dot com>
- To: Yao Qi <qiyaoltc at gmail dot com>
- Cc: Philipp Rudo <prudo at linux dot vnet dot ibm dot com>, "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>, nd <nd at arm dot com>
- Date: Thu, 25 Jan 2018 16:13:16 +0000
- Subject: Re: [PATCH v2 6/8] Create xml from target descriptions
- Authentication-results: sourceware.org; auth=none
- Authentication-results: spf=none (sender IP is ) smtp.mailfrom=Alan dot Hayward at arm dot com;
- Nodisclaimer: True
- References: <7C97CC6A-92CB-4702-820D-206022F07102@arm.com> <B741D224-2DE6-42FD-8D32-59D68205EB6E@arm.com> <20180125141423.1ff34025@ThinkPad> <86vafqc58o.fsf@gmail.com>
- Spamdiagnosticmetadata: NSPM
- Spamdiagnosticoutput: 1:99
> On 25 Jan 2018, at 13:14, Philipp Rudo <prudo@linux.vnet.ibm.com> wrote:
>
>
> i don't really like print_xml_feature::visit_pre being declared in arch/tdesc.h
> with different implementations in target-descriptions.c and gdbserver/tdesc.c
> while all other visit_* implementations are in arch/tdesc.c. I would prefere
> visit_pre being implemented in arch/tdesc.c, too. Even when it means to add
> some #ifdef GDBSERVER ... #else ... #endif blocks until there is a common tdesc.
Thanks for the review!
A common print_xml_feature would require 4 ifdefs, which would be too messy.
I’ll take a look see if I can communise the tdesc_architecture and tdesc_osabi
interfaces - it should be fairly simple to do.
Once done, I should be able to add a common print_xml_feature without any
ifdefs. Hopefully.
> On 25 Jan 2018, at 15:44, Yao Qi <qiyaoltc@gmail.com> wrote:
>
> Philipp Rudo <prudo@linux.vnet.ibm.com> writes:
>
>> Same for tdesc_type::make_gdb_type (patch #5). But here i would prefer to not
>> even declare the method for GDBserver, i.e.
>>
>> struct tdesc_type
>> {
>>
>> [...]
>>
>> #ifndef GDBSERVER
>> virtual type *make_gdb_type (struct gdbarch *gdbarch) const = 0;
>> #endif
>> };
>>
>> The problem i see with implementing stubs calling error is that you cannot find
>> out you made a mistake until you call the function during run-time. This gives
>> room to nasty bugs which could easily be prevented when there is a compile bug.
>
>
> make_gdb_type and gdbarch shouldn't be put into arch/tdesc.h at all, if
> possible. You can create an sub-class of tdesc_element_visitor in gdb
> side, and create the gdb type by visiting these elements, like this,
>
> class gdb_type_creator : public tdesc_element_visitor
….
Nice idea. I can add an extra patch into the series to do this.