Re: [PATCH 3/5] Refactor in mips-linux-nat.c

["Maciej W. Rozycki" <macro at codesourcery dot com>]

On Sat, 29 Jun 2013, Yao Qi wrote:

> 2013-06-29  Yao Qi  <yao@codesourcery.com>
> 
> 	* mips-linux-nat.c (get_irw_mask): Rename to ...
> 	(mips_linux_watch_get_irw_mask): ... this.  Rename parameter
> 	'set' to 'n'.  Update function comment.  All callers changed.
> 	(get_reg_mask): Rename parameter 'set' to 'n'.  Update function
> 	comment.  All callers changed.
> 	(get_num_valid): Rename to ...
> 	(mips_linux_watch_get_num_valid): ... this.  Rename parameter
> 	'set' to 'n'.  Update function comment.  All callers changed.
> 	(get_watchlo): Rename to ...
> 	(mips_linux_watch_get_watchlo): ... this.  Rename parameter 'set'
> 	to 'n'.  Update function comment.  All callers changed.
> 	(set_watchlo): Rename to ...
> 	(mips_linux_watch_set_watchlo): ... this.  Rename parameter 'set'
> 	to 'n'.  Update function comment.  All callers changed.
> 	(get_watchhi): Rename to ...
> 	(mips_linux_watch_get_watchhi): ... this.  Update function
> 	comment.  All callers changed.
> 	(set_watchhi): Rename to ...
> 	(mips_linux_watch_set_watchhi): ... this.  Update function
> 	comment.  All callers changed.
> 	(mips_linux_read_watch_registers): Update function comment.
> 	Add new parameters 'lwpid', 'watch_readback', and
> 	'watch_readback_valid'.  Update.
> 	(type_to_irw): Rename to ...
> 	(mips_linux_watch_type_to_irw): ... this.  Update function
> 	comment.  All callers changed.
> 	(fill_mask): Update function comment.
> 	(try_one_watch): Rename to ...
> 	(mips_linux_watch_try_one_watch): ... this.  Change the type of
> 	parameter 'irw' from 'unsigned' to 'uint32_t'.
> 	(populate_regs_from_watches): Rename to ...
> 	(mips_linux_watch_populate_regs): ... this.  Add parameter
> 	'current_watches'.  All callers changed.
> ---
>  gdb/mips-linux-nat.c |  208 +++++++++++++++++++++++++++++---------------------
>  1 files changed, 121 insertions(+), 87 deletions(-)
> 
> diff --git a/gdb/mips-linux-nat.c b/gdb/mips-linux-nat.c
> index 44d23a6..af70945 100644
> --- a/gdb/mips-linux-nat.c
> +++ b/gdb/mips-linux-nat.c
> @@ -874,29 +892,31 @@ try_one_watch (struct pt_watch_regs *regs, CORE_ADDR addr,
>    base_addr = addr & ~mask_bits;
>  
>    /* Check to see if it is covered by current registers.  */
> -  for (i = 0; i < get_num_valid (regs); i++)
> +  for (i = 0; i < mips_linux_watch_get_num_valid (regs); i++)
>      {
> -      t_low = get_watchlo (regs, i);
> -      if (t_low != 0 && irw == ((unsigned)t_low & irw))
> +      t_low = mips_linux_watch_get_watchlo (regs, i);
> +      if (t_low != 0 && irw == ((uint32_t) t_low & irw))
>  	{
> -	  t_hi = get_watchhi (regs, i) | IRW_MASK;
> -	  t_low &= ~(CORE_ADDR)t_hi;
> +	  t_hi = mips_linux_watch_get_watchhi (regs, i) | IRW_MASK;
> +	  t_low &= ~(CORE_ADDR) t_hi;
>  	  if (addr >= t_low && last_byte <= (t_low + t_hi))
>  	    return 1;
>  	}
>      }
>    /* Try to find an empty register.  */
>    free_watches = 0;
> -  for (i = 0; i < get_num_valid (regs); i++)
> +  for (i = 0; i < mips_linux_watch_get_num_valid (regs); i++)
>      {
> -      t_low = get_watchlo (regs, i);
> -      if (t_low == 0 && irw == (get_irw_mask (regs, i) & irw))
> +      t_low = mips_linux_watch_get_watchlo (regs, i);
> +      if (t_low == 0
> +	  && irw == (mips_linux_watch_get_irw_mask (regs, i) & irw))
>  	{
>  	  if (mask_bits <= (get_reg_mask (regs, i) | IRW_MASK))
>  	    {
>  	      /* It fits, we'll take it.  */
> -	      set_watchlo (regs, i, base_addr | irw);
> -	      set_watchhi (regs, i, mask_bits & ~IRW_MASK);
> +	      mips_linux_watch_set_watchlo (regs, i, base_addr | irw);
> +	      mips_linux_watch_set_watchhi (regs, i,
> +					    mask_bits & ~IRW_MASK);

 This fits on one line, please adjust.

> @@ -910,21 +930,25 @@ try_one_watch (struct pt_watch_regs *regs, CORE_ADDR addr,
>      {
>        /* Try to split it across several registers.  */
>        regs_copy = *regs;
> -      for (i = 0; i < get_num_valid (&regs_copy); i++)
> +      for (i = 0;
> +	   i < mips_linux_watch_get_num_valid (&regs_copy);
> +	   i++)

 Likewise.  OK with these changes, thanks.

  Maciej