This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Add OSE operating system support [5/5] gdbarch support
- From: Tom Tromey <tromey at redhat dot com>
- To: Hui Zhu <hui_zhu at mentor dot com>
- Cc: <gdb-patches at sourceware dot org>, <binutils at sourceware dot org>, <lgustavo at codesourcery dot com>, Eli Zaretskii <eliz at gnu dot org>
- Date: Fri, 19 Apr 2013 13:02:23 -0600
- Subject: Re: [PATCH] Add OSE operating system support [5/5] gdbarch support
- References: <5135C146 dot 1030303 at mentor dot com> <5135C1A7 dot 7020005 at mentor dot com> <5135C200 dot 7080006 at mentor dot com> <5135C26C dot 7000808 at mentor dot com> <5135C2D1 dot 5090503 at mentor dot com> <5139E7B4 dot 4090601 at mentor dot com>
>>>>> "Hui" == Hui Zhu <hui_zhu@mentor.com> writes:
Hui> 2013-03-08 Pedro Alves <pedro@codesourcery.com>
Hui> Luis Machado <lgustavo@codesourcery.com>
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> +#define PM_SECTION_NAME_LENGTH PM_STRING_LENGTH
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.
Tom