This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: RFA - PATCH - Support H8/300H and H8S Normal Mode for GDB
- From: Corinna Vinschen <vinschen at redhat dot com>
- To: gdb-patches at sources dot redhat dot com
- Date: Mon, 22 Sep 2003 19:27:17 +0200
- Subject: Re: RFA - PATCH - Support H8/300H and H8S Normal Mode for GDB
- References: <69595093233BB547BB70CF5E492B63F201F7240B@sohm.kpit.com>
- Reply-to: gdb-patches at sources dot redhat dot com
On Mon, Sep 15, 2003 at 02:25:20PM +0530, Shrinivas Atre wrote:
> Hi,
>
> The attached patch enables debugging and simulation of H8/300H and H8S
> Normal Mode binaries in GDB.
>
> Is that OK for mainline and 6.0 branch ?
Do you have an FSF copyright assignment? AFAIK you need that.
> --- src/gdb/config/h8300/tm-h8300.h.orig Mon Sep 15 10:46:23 2003
> +++ src/gdb/config/h8300/tm-h8300.h Mon Sep 15 13:32:23 2003
> @@ -26,6 +26,7 @@
> GDB_TARGET_IS_H8300 in remote-e7000.c */
> extern int h8300hmode;
> extern int h8300smode;
> +extern int NormalMode; /* 1 - Normal Mode , 0 - Advanced mode */
Please don't use UpperCase but only lower case and underscore for
variable names. This variable should be named normal_mode.
In this very case, the variable is extern so the name should be much
less ambiguous. Probably "h8300_normal_mode" would fit.
> - case bfd_mach_h8300h:
> case bfd_mach_h8300hn:
> + NormalMode = 1;
> + h8300sxmode = 0;
> + h8300smode = 0;
> + h8300hmode = 1;
> + set_gdbarch_num_regs (gdbarch, 13);
> + set_gdbarch_num_pseudo_regs (gdbarch, 1);
> + set_gdbarch_ecoff_reg_to_regnum (gdbarch, h8300_dbg_reg_to_regnum);
> + set_gdbarch_dwarf_reg_to_regnum (gdbarch, h8300_dbg_reg_to_regnum);
> + set_gdbarch_dwarf2_reg_to_regnum (gdbarch, h8300_dbg_reg_to_regnum);
> + set_gdbarch_stab_reg_to_regnum (gdbarch, h8300_dbg_reg_to_regnum);
> + set_gdbarch_register_name (gdbarch, h8300_register_name);
> + set_gdbarch_ptr_bit (gdbarch, 2 * TARGET_CHAR_BIT);
> + set_gdbarch_addr_bit (gdbarch, 2 * TARGET_CHAR_BIT);
> + set_gdbarch_extract_return_value (gdbarch, h8300h_extract_return_value);
> + set_gdbarch_store_return_value (gdbarch, h8300h_store_return_value);
> + set_gdbarch_print_insn (gdbarch, print_insn_h8300h);
> + break;
> + case bfd_mach_h8300h:
> + NormalMode = 0;
> h8300sxmode = 0;
> h8300smode = 0;
> h8300hmode = 1;
Is it necessary to duplicate all set_gdbarch_XXX calls? Wouldn't
it be more clear to set "h8300_normal_mode" separately to show that
it's the same machine, just another mode? E. g.
case bfd_mach_h8300h:
case bfd_mach_h8300hn:
if (info.bfd_arch_info->mach)
h8300_normal_mode = 1;
> @@ -510,6 +510,7 @@ enum { POLL_QUIT_INTERVAL = 0x80000 };
>
> int h8300hmode = 0;
> int h8300smode = 0;
> +int NormalMode = 0;
As above, "h8300_normal_mode" would be better.
Otherwise it looks ok but I guess we need a word from the h8300 sim
expert. Michael?
Corinna
--
Corinna Vinschen
Cygwin Developer
Red Hat, Inc.