Re: [RFA 3/5] New port: CR16: gdb port

[Yao Qi <yao@codesourcery.com>]

On 10/04/2012 06:20 PM, Kaushik Phatak wrote:

Hi, Kaushik,
I don't read your patch in details yet, just some small points I found...

> +const gdb_byte breakpoint_uclinux[] = { 0xC7, 0x00 };

Add 'static'.  It is not used out of this file.

> +/* Allocate and initialize a gdbarch object.  */
> +static struct gdbarch *
> +cr16_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> +{
> +  struct gdbarch *gdbarch;
> +  struct gdbarch_tdep *tdep;
> +  int elf_flags;
> +
> +  /* Extract the elf_flags if available.  */
> +  if (info.abfd != NULL
> +      && bfd_get_flavour (info.abfd) == bfd_target_elf_flavour)
> +    elf_flags = elf_elfheader (info.abfd)->e_flags;
> +  else
> +    elf_flags = 0;
> +
> +  /* Try to find the architecture in the list of already defined
> +     architectures.  */
> +  for (arches = gdbarch_list_lookup_by_info (arches, &info);
> +       arches != NULL;
> +       arches = gdbarch_list_lookup_by_info (arches->next, &info))
> +    {
> +      if (gdbarch_tdep (arches->gdbarch)->elf_flags != elf_flags)
> +	continue;
> +
> +      return arches->gdbarch;
> +    }
> +  /* None found, create a new architecture from the information
> +     provided.  */
> +  tdep = (struct gdbarch_tdep *) xcalloc (1,sizeof (struct gdbarch_tdep));
> +  tdep->elf_flags = elf_flags;
> +  gdbarch = gdbarch_alloc (&info, tdep);
> +
> +  set_gdbarch_num_pseudo_regs (gdbarch, 0);
> +  if(info.osabi == GDB_OSABI_LINUX)
> +  {
> +    set_gdbarch_num_regs (gdbarch, CR16_LINUX_NUM_REGS);
> +    set_gdbarch_register_name (gdbarch, cr16_linux_register_name);

This part should be moved to function cr16_uclinux_init_abi, and then 
macro CR16_LINUX_NUM_REGS can be defined in cr16-linux-tdep.c instead of 
cr16-tdep.h.

> +  }
> +  else
> +  {
> +    set_gdbarch_num_regs (gdbarch, CR16_NUM_REGS);
> +    set_gdbarch_register_name (gdbarch, cr16_register_name);
> +  }

> --- ./gdb_src.orig/gdb/cr16-tdep.h	1970-01-01 05:30:00.000000000 +0530
> +++ ./gdb_src/gdb/cr16-tdep.h	2012-09-04 13:05:39.000000000 +0530
> @@ -0,0 +1,36 @@
> +/* GNU/Linux on  CR16 target support.
> +   Copyright (C) 2011-2012 Free Software Foundation, Inc.
> +
> +   Contributed by Kaushik Phatak (kaushik.pahatk@kpitcummins.com)
> +   KPIT Cummins Infosystems Limited, Pune India.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see<http://www.gnu.org/licenses/>.  */
> +
> +#define CR16_NUM_REGS  22
> +#define CR16_LINUX_NUM_REGS  21

These two macros can be defined in cr16-tdep.c and cr16-linux-tdep.c 
respectively.

> +
> +extern const gdb_byte breakpoint_elf[];
> +extern const gdb_byte breakpoint_linux[];

They are not needed.  'breakpoint_elf' is only used in cr16-tdep.c, and 
'breakpoint_linux' doesn't exist at all.

-- 
Yao