This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] MIPS SDE OS ABI support
- From: Joel Brobecker <brobecker at adacore dot com>
- To: Yao Qi <yao at codesourcery dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Mon, 15 Dec 2014 08:10:32 -0500
- Subject: Re: [PATCH] MIPS SDE OS ABI support
- Authentication-results: sourceware.org; auth=none
- References: <1418629607-20086-1-git-send-email-yao at codesourcery dot com>
> 2014-12-15 Maciej W. Rozycki <macro@codesourcery.com>
> Nigel Stephens <nigel@mips.com>
> Chris Dearman <chris@mips.com>
> Luis Machado <lgustavo@codesourcery.com>
>
> * sde-mips-tdep.c: New file containg SDE specific code.
> * configure.tgt (mips*-sde*-elf*): Add sde-mips-tdep.o to
> gdb_target_obs.
> * defs.h (gdb_osabi): Add GDB_OSABI_SDE.
> * osabi.c (gdb_osabi_names): Add SDE.
> * NEWS: Mention the change.
I just skimmed through that patch, and noticed a few things, all
of fairly trivial nature. So, as soon as fixed, I think the patch
can go in (modulo NEWS to be approved by Eli).
> +static struct trad_frame_cache *
> +mips_sde_frame_cache (struct frame_info *this_frame, void **this_cache)
All functions should have an introductory comment. Since these
implement callbacks/"methods", the usual one-liner that describe
which callback they implement should be used.
> +static void
> +mips_sde_frame_this_id (struct frame_info *this_frame, void **this_cache,
> + struct frame_id *this_id)
> +{
> + struct trad_frame_cache *this_trad_cache
> + = mips_sde_frame_cache (this_frame, this_cache);
> + trad_frame_get_id (this_trad_cache, this_id);
Empty line between variable declaration and the rest of the code.
> +static struct value *
> +mips_sde_frame_prev_register (struct frame_info *this_frame,
> + void **this_cache,
> + int prev_regnum)
> +{
> + struct trad_frame_cache *trad_cache
> + = mips_sde_frame_cache (this_frame, this_cache);
> + return trad_frame_get_register (trad_cache, this_frame, prev_regnum);
Likewise.
> +static CORE_ADDR
> +mips_sde_frame_base_address (struct frame_info *this_frame, void **this_cache)
> +{
> + struct trad_frame_cache *this_trad_cache
> + = mips_sde_frame_cache (this_frame, this_cache);
> + return trad_frame_get_this_base (this_trad_cache);
Same here.
--
Joel