> On 25 Jan 2018, at 13:14, Philipp Rudo 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 wrote: > > Philipp Rudo 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. &j!z޶׎tIb֫rnr