From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1L0oM7-0005lE-8S for qemu-devel@nongnu.org; Thu, 13 Nov 2008 21:24:19 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1L0oM5-0005ki-Nr for qemu-devel@nongnu.org; Thu, 13 Nov 2008 21:24:17 -0500 Received: from [199.232.76.173] (port=40491 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1L0oM5-0005kf-HK for qemu-devel@nongnu.org; Thu, 13 Nov 2008 21:24:17 -0500 Received: from mail2.shareable.org ([80.68.89.115]:50392) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1L0oM5-0000pX-8n for qemu-devel@nongnu.org; Thu, 13 Nov 2008 21:24:17 -0500 Date: Fri, 14 Nov 2008 02:24:12 +0000 From: Jamie Lokier Subject: Re: [Qemu-devel] Re: [PATCH 03/12] Refactor and enhance break/watchpoint API Message-ID: <20081114022412.GE2055@shareable.org> References: <20081103103558.213902776@mchn012c.ww002.siemens.net> <20081103103559.071243441@mchn012c.ww002.siemens.net> <491C9293.2090603@codemonkey.ws> <491CB0B2.1040701@web.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <491CB0B2.1040701@web.de> Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Jan Kiszka 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