This is the mail archive of the mailing list for the binutils 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]

Re: [PATCH] Add OSE operating system support [5/5] gdbarch support

>>>>> "Hui" == Hui Zhu <> writes:

Hui> 2013-03-08  Pedro Alves  <>
Hui> 	    Luis Machado  <>
Hui> 	* configure.tgt: Add powerpc-*-ose.
Hui> 	* defs.h (gdb_osabi): Add GDB_OSABI_OSE.
Hui> 	* osabi.c (gdb_osabi_name): Add "OSE".
Hui> 	* ose-tdep.c: New.

This patch looks pretty good.
I just have a few small notes.

Hui> +
Hui> +#define PM_STRING_LENGTH             32
Hui> +
Hui> +
Hui> +#define PM_SECTION_LOAD 1
Hui> +

There are lots of un-commented things in the patch.
I think it could use more comments generally.

Hui> +static int
Hui> +fill_register_ids_array (int regnum, MonitorRegisterId *array)
Hui> +{
Hui> +  if (regnum == -1)
Hui> +    {
Hui> +      gdb_assert (0);
Hui> +    }

We use gdb_assert_not_reached now, but better would be just:

  gdb_assert (regnum != -1);
  ... remove the 'else' and outdent that whole block here ...

Hui> +#if 0
Hui> +      warning ("no mapping for register %d", regnum);
Hui> +#endif
Hui> +      return 0;
Hui> +      internal_error (__FILE__, __LINE__,
Hui> +		      "unknown regnum %d", regnum);

The internal_error is dead code.
Please remove the #if 0 code as well.

Hui> +static const struct regset *
Hui> +ose_regset_from_core_section (struct gdbarch *gdbarch,
Hui> +			      const char *sect_name, size_t sect_size)
Hui> +{
Hui> +  /* FIXME: check section size.  */
Hui> +  if (strcmp (sect_name, ".reg") == 0)
Hui> +    return &ose_gregset;

If it needs a check can you add it?
Or else, I suppose, just delete the comment.

Hui> +static void
Hui> +ose_core_relocate (struct gdbarch *gdbarch, bfd *core_bfd)
Hui> +{
Hui> +  struct load_map_info info = { NULL, NULL };
Hui> +  asection *asect;
Hui> +  struct PmLoadModuleSectionInfoReply *lmsi;
Hui> +  int i;
Hui> +  int start, end;
Hui> +
Hui> +  asect = bfd_get_section_by_name (core_bfd, ".section-info");
Hui> +
Hui> +  lmsi = (void *) asect->contents;

It seems like this should use bfd_get_section_contents.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]