On 2 October 2013 00:13, Richard Henderson <r...@twiddle.net> wrote: > On 10/01/2013 07:28 AM, Fabien Chouteau wrote: >> On 10/01/2013 04:00 AM, Richard Henderson wrote: >>> On 09/30/2013 08:57 AM, Fabien Chouteau wrote: >>>> +extern const MonitorDef arch_monitor_defs[]; >>> >>> This is supplied by target-foo/monitor.c, right? >>> Why in the world is it declared in generic code? >>> >> >> Yes, why? >> >>> Especially if it's only ever accessed via the >>> cpu->monitor_defs member? >>> >> >> To begin with, I though I'd put in in each target-*/cpu.c, then having >> it at only one place seemed more clean. I'm open to any suggestion. >> > > If you put it in a global header like that, it looks like the array is > part of the generic interface. But it isn't -- only the monitor_defs > member is. > > If it's declared in any header at all, as opposed to static and local > to cpu.c (why was a new monitor.c file chosen?)
Keeps monitor specific stuff in one file for the target and avoids cpu.c gradually bloating into something enormous. Compare use of gdbstub.c for the gdb stub related target specific functions. The declaration should go in cpu-qom.h (again, compare the gdb stuff), and it should be called "arm_monitor_defs", "ppc_monitor_defs" etc, not arch_monitor_defs. (This avoids issues if we ever manage to compile more than one target CPU into a single qemu binary, and again, it follows existing conventions). -- PMM
