Re: [PATCH 4/5] Move mips hardware watchpoint stuff to common/

Maciej W. Rozycki
2013-07-24T00:31:06Z

On Sat, 29 Jun 2013, Yao Qi wrote:

> 2013-06-29 Yao Qi <yao@codesourcery.com>
>
> * Makefile.in ((HFILES_NO_SRCDIR): Add common/mips-linux-watch.h.
> (mips-linux-watch.o): New rule.
> * common/mips-linux-watch.c: New.
> * common/mips-linux-watch.h: New.
> * config/mips/linux.mh (NATDEPFILES): Add mips-linux-watch.o
> * mips-linux-nat.c: Don't include asm/ptrace. Include
> mips-linux-watch.h.
> (W_BIT, R_BIT, I_BIT, W_MASK, R_MASK, I_MASK, IRW_MASK): Move
> to common/mips-linux-watch.h.
> (MAX_DEBUG_REGISTER): Likewise.
> (enum pt_watch_style): Likewise.
> (struct mips32_watch_regs): Likewise.
> (struct mips64_watch_regs): Likewise.
> (struct pt_watch_regs): Likewise.
> (struct mips_watchpoint): Likewise.
> (mips_linux_watch_get_irw_mask): Move to
> common/mips-linux-watch.c.
> (get_reg_mask, mips_linux_watch_get_num_valid): Likewise.
> (mips_linux_watch_get_watchlo): Likewise.
> (mips_linux_watch_set_watchlo): Likewise.
> (mips_linux_watch_get_watchhi): Likewise.
> (mips_linux_watch_set_watchhi): Likewise.
> (mips_linux_read_watch_registers): Likewise.
> (mips_linux_watch_type_to_irw): Likewise.
> (mips_linux_stopped_data_address, fill_mask): Likewise.
> (mips_linux_watch_try_one_watch): Likewise.
> (mips_linux_watch_populate_regs): Likewise.
> ---
> gdb/Makefile.in | 6 +-
> gdb/common/mips-linux-watch.c | 352 ++++++++++++++++++++++++++++++++++
> gdb/common/mips-linux-watch.h | 129 +++++++++++++
> gdb/config/mips/linux.mh | 2 +-
> gdb/mips-linux-nat.c | 414 +----------------------------------------
> 5 files changed, 489 insertions(+), 414 deletions(-)
> create mode 100644 gdb/common/mips-linux-watch.c
> create mode 100644 gdb/common/mips-linux-watch.h
>
> diff --git a/gdb/common/mips-linux-watch.c b/gdb/common/mips-linux-watch.c
> new file mode 100644
> index 0000000..f107530
> --- /dev/null
> +++ b/gdb/common/mips-linux-watch.c
> @@ -0,0 +1,352 @@
> +/* Copyright (C) 2009-2013 Free Software Foundation, Inc.

The copyright years don't appear right to me. Either 2001-2013, taken
from gdb/mips-linux-nat.c, or just lone 2013 seem more appropriate to me,
but http://www.gnu.org/prep/maintain/maintain.html does not appear clear
to me on it -- would someone please step in and explain what GDB's usual
policy has been here?

Can you please reorder the sequence of functions moved here to match the
original one in gdb/mips-linux-nat.c?

Also, anyone: do we still aim to maintain gdb/config/djgpp/fnchange.lst?
If so, then on what basis -- ad hoc? ChangeLogs show most recent random
additions last year only and files appear to be missing from the listing.

> diff --git a/gdb/common/mips-linux-watch.h b/gdb/common/mips-linux-watch.h
> new file mode 100644
> index 0000000..4f91183
> --- /dev/null
> +++ b/gdb/common/mips-linux-watch.h
> @@ -0,0 +1,129 @@
> +/* Copyright (C) 2009-2013 Free Software Foundation, Inc.

Same copyright year concern as above.

> diff --git a/gdb/mips-linux-nat.c b/gdb/mips-linux-nat.c
> index af70945..27467f4 100644
> --- a/gdb/mips-linux-nat.c
> +++ b/gdb/mips-linux-nat.c
> @@ -34,7 +34,8 @@
>
> #include <sgidefs.h>
> #include <sys/ptrace.h>
> -#include <asm/ptrace.h>
> +
> +#include "mips-linux-watch.h"

You actually need to keep that <asm/ptrace.h> inclusion, per GDB's coding
requirements (see gdbint.info).

Please resend with the changes requested.

Maciej