This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
Hi Pedro, On 21.01.13 16:49, Pedro Alves wrote: > On 12/27/2012 09:36 PM, Andreas Tobler wrote: > >> in order to avoid code duplication for the FreeBSD powerpc port I >> started to cut off common code from ppc-linux-tdep.c into a new file to >> be used for FreeBSD and GNU/Linux PowerPC 64-bit. The file name is open >> so far. Better namings are welcome. > > Thanks for doing this. > > IMO, the "common" monitor is practically meaningless, and raises the question > of what's different between ppc64-common-tdep.c, and > an hypothetical ppc64-tdep.c, and/or the rs6000-tdep.c, as the > second would be about common ppc64 bits, and the latter, extant, _is also_ > about common ppc bits. IOW, if we need a new ppc function in the future that's > be used by several OSs, how do we decide where it goes? "common" doesn't > give any clue. So I'd suggest ppc64-linbsd-tdep.c, and/or > rs6000-tdep.c/ppc64-tdep.c, for shared code that is not OS specific (yeah, > rs6000-tdep.c is a misnomer nowadays). First, thank you for the review. I attached a revised version of the patch including the CL below. I hope I matched your points. I decided to leave ppc64-linbsd-tdep.c away and use ppc64-tdep.c|h with the changes mentioned below to ppc-tdep.h/rs6000-tdep.c Tested on ppc64-linux and on FreeBSD amd64 with an --enable-targets=all. >> +/* An instruction to match. */ >> + >> +struct insn_pattern > ... >> + >> +int >> +insns_match_pattern (CORE_ADDR pc, struct insn_pattern *pattern, >> + unsigned int *insn); >> +CORE_ADDR >> +insn_d_field (unsigned int insn); > > These look like basic architecture/instruction pattern matching. > I suggest moving to ppc-tdep.h/rs6000-tdep.c instead. Done. > If we're making these extern, then it'd be good to add a "ppc_" or > "ppc64_" prefix (could be a separate step). Done with ppc_ since the instructions could be used on ppc and ppc64 > Also, GDB's convention is that the function name goes on the first > column in function definitions only, not declarations, and that > declarations in .h files get an explicit "extern". Done, not sure if I need the extern on ppc64-tdep.h too? > Otherwise looks fine to me. Again, thank you! Andreas 2013-01-26 Andreas Tobler <andreast@fgznet.ch> * Makefile.in (ALL_TARGET_OBS): Add new file ppc64-tdep.o. (HFILES_NO_SRCDIR): Likewise. (ALLDEPFILES): Likewise. * configure.tgt: Add new file for powerpc-linux. * ppc64-tdep.h: New file. * ppc64-tdep.c: New file. (insn_d, insn_ds, insn_xfx, ppc64_desc_entry_point): Move from ppc-linux-tdep.c to here. (PPC64_STANDARD_LINKAGE1_LEN, PPC64_STANDARD_LINKAGE2_LEN) (PPC64_STANDARD_LINKAGE2_LEN): Likewise and use ARRAY_SIZE macro. (ppc64_standard_linkage1_target, ppc64_standard_linkage2_target) (ppc64_standard_linkage3_target, ppc64_skip_trampoline_code): Move from ppc-linux-tdep.c to here. (ppc64_convert_from_func_ptr_addr): Rename it from ppc64_linux_convert_from_func_ptr_addr to ppc64_convert_from_func_ptr_addr and move it from ppc-linux-tdep.c to here. * rs6000-tdep.c: (read_insn): Move from ppc-linux-tdep.c to here. (insns_match_pattern, insn_d_field, insn_ds_field): Move from ppc-linux-tdep.c to here and rename them with the ppc_ prefix. * ppc-linux-tdep.c: Include ppc64-tdep.h. Removed above functions. (ppc_linux_init_abi): Rename ppc64_linux_convert_from_func_ptr_addr to ppc64_convert_from_func_ptr_addr.
Attachment:
ppc64_tdep_1diff
Description: Text document
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |