Re: [PATCH 07/13] Split brekapoint_from_pc to breakpoint_kind_from_pc and sw_breakpoint_from_kind

[Pedro Alves <palves at redhat dot com>]

Note the "brekapoint" typo -- it appears through the series
several times, both in code and in email subjects.  I suggest
using git format-patch, and grep the patches to find them all.

On 08/31/2016 04:05 PM, Yao Qi wrote:

> +static const gdb_byte *
> +mips_sw_breakpoint_from_kind (struct gdbarch *gdbarch, int kind, int *size)
> +{



> +    case MIPS_BP_KIND_32BIT:
> +      {
> +	/* The IDT board uses an unusual breakpoint value, and
> +	   sometimes gets confused when it sees the usual MIPS
> +	   breakpoint instruction.  */
> +	static gdb_byte big_breakpoint[] = { 0, 0x5, 0, 0xd };
> +	static gdb_byte little_breakpoint[] = { 0xd, 0, 0x5, 0 };
> +	static gdb_byte pmon_big_breakpoint[] = { 0, 0, 0, 0xd };
> +	static gdb_byte pmon_little_breakpoint[] = { 0xd, 0, 0, 0 };
> +	static gdb_byte idt_big_breakpoint[] = { 0, 0, 0x0a, 0xd };
> +	static gdb_byte idt_little_breakpoint[] = { 0xd, 0x0a, 0, 0 };
> +	/* Likewise, IRIX appears to expect a different breakpoint,
> +	   although this is not apparent until you try to use pthreads.  */
> +	static gdb_byte irix_big_breakpoint[] = { 0, 0, 0, 0xd };
> +
> +	*size = 4;
> +
> +	if (strcmp (target_shortname, "mips") == 0)
> +	  {
> +	    if (byte_order_for_code == BFD_ENDIAN_BIG)
> +	      return idt_big_breakpoint;
> +	    else
> +	      return idt_little_breakpoint;
> +	  }
> +	else if (strcmp (target_shortname, "ddb") == 0
> +		 || strcmp (target_shortname, "pmon") == 0
> +		 || strcmp (target_shortname, "lsi") == 0)
> +	  {
> +	    if (byte_order_for_code == BFD_ENDIAN_BIG)
> +	      return pmon_big_breakpoint;
> +	    else
> +	      return pmon_little_breakpoint;
> +	  }
> +	else if (gdbarch_osabi (gdbarch) == GDB_OSABI_IRIX)
> +	  return irix_big_breakpoint;

FYI, I think all of these above could be removed. 
target mips/ddb/pmon/etc. are all gone, and we don't support IRIX any
longer either:

*** Changes in GDB 7.12*** Changes in GDB 7.12

* Support for various remote target protocols and ROM monitors has
  been removed:

  target m32rsdi        Remote M32R debugging over SDI
  target mips           MIPS remote debugging protocol
  target pmon           PMON ROM monitor
  target ddb            NEC's DDB variant of PMON for Vr4300
  target rockhopper     NEC RockHopper variant of PMON
  target lsi            LSI variant of PMO

*** Changes in GDB 7.10

Support for these obsolete configurations has been removed.

SGI Irix-5.x                            mips-*-irix5*
SGI Irix-6.x                            mips-*-irix6*

> +	else
> +	  {
> +	    if (byte_order_for_code == BFD_ENDIAN_BIG)
> +	      return big_breakpoint;
> +	    else
> +	      return little_breakpoint;
> +	  }
> +      }
> +    default:
> +      gdb_assert_not_reached ("unexpected mips breakpoint kind");
> +    };
> +}
> +
> +/* This function implements gdbarch_breakpoint_from_pc.  It uses the
> +   program counter value to determine whether a 16- or 32-bit breakpoint
> +   should be used.  It returns a pointer to a string of bytes that encode a
> +   breakpoint instruction, stores the length of the string to *lenptr, and
> +   adjusts pc (if necessary) to point to the actual memory location where
> +   the breakpoint should be inserted.  */

I see this comment repeated in several places.  Shouldn't we replace it
with some "implement foo" comment ?

/me reads rest of series...

Oh, this is all eliminated in the end.  So nevermind.

> +
> +GDBARCH_BREAKPOINT_FROM_PC (mips)
> +


Thanks,
Pedro Alves