Re: [PATCH] MIPS SDE OS ABI support

[Joel Brobecker <brobecker@adacore.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