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

Maciej W. Rozycki
2013-07-24T00:27:12Z

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