[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] Re: [PATCH 03/12] Refactor and enhance break/watchpoint
From: |
Jamie Lokier |
Subject: |
Re: [Qemu-devel] Re: [PATCH 03/12] Refactor and enhance break/watchpoint API |
Date: |
Fri, 14 Nov 2008 02:24:12 +0000 |
User-agent: |
Mutt/1.5.13 (2006-08-11) |
Jan Kiszka wrote:
> Anthony Liguori wrote:
> > Jan Kiszka wrote:
> >> Index: b/gdbstub.c
> >> ===================================================================
> >> --- a/gdbstub.c
> >> +++ b/gdbstub.c
> >> @@ -1145,10 +1145,64 @@ void gdb_register_coprocessor(CPUState *
> >> }
> >> }
> >>
> >> +/* GDB breakpoint/watchpoint types */
> >> +#define GDB_BREAKPOINT_SW 0
> >> +#define GDB_BREAKPOINT_HW 1
> >> +#define GDB_WATCHPOINT_WRITE 2
> >> +#define GDB_WATCHPOINT_READ 3
> >> +#define GDB_WATCHPOINT_ACCESS 4
> >> +
> >> +#ifndef CONFIG_USER_ONLY
> >> +static const int xlat_gdb_type[] = {
> >> + [GDB_WATCHPOINT_WRITE] = BP_GDB | BP_MEM_WRITE,
> >> + [GDB_WATCHPOINT_READ] = BP_GDB | BP_MEM_READ,
> >> + [GDB_WATCHPOINT_ACCESS] = BP_GDB | BP_MEM_ACCESS,
> >> +};
> >> +#endif
> >> +
> >> +static int gdb_breakpoint_insert(CPUState *env, target_ulong addr,
> >> + target_ulong len, int type)
> >> +{
> >> + switch (type) {
> >> + case GDB_BREAKPOINT_SW ... GDB_BREAKPOINT_HW:
> >>
> >
> > We've avoided this GCCism pretty much. I don't think the code is
> > significantly cleaner with it so I think it's best to avoid it.
>
> OK, I see the general problem. Restricting ourselves here is not a big
> issue - but for my other series which tried hard to canonicalizes x86's
> cpu_gdb_read/write_register, sigh...
I think the code would be clearer with separate cases in this instance anyway.
Usuallywhen you have an enumeration which is just labels without a
numerical purpose, seeing "..." or "<" on it is not clear which cases
are included, and it tends to break quietly when someone adds more values.
If there's a long sequence of cases which crops up often, then you can
use a macro "#define cases_FOO case FOO1: case FOO2: case FOO3:".
-- Jamie
[Qemu-devel] [PATCH 04/12] Set mem_io_vaddr on io_read, Jan Kiszka, 2008/11/03
[Qemu-devel] [PATCH 06/12] Restore pc on watchpoint hits, Jan Kiszka, 2008/11/03
[Qemu-devel] [PATCH 11/12] Introduce BP_CPU as a breakpoint type, Jan Kiszka, 2008/11/03
[Qemu-devel] [PATCH 08/12] qemu: gdbstub: manage CPUs as threads, Jan Kiszka, 2008/11/03
[Qemu-devel] [PATCH 09/12] Introduce BP_WATCHPOINT_HIT flag, Jan Kiszka, 2008/11/03
[Qemu-devel] [PATCH 07/12] Remove premature memop TB terminations, Jan Kiszka, 2008/11/03