qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 3/4] Refactoring MonitorDef array


From: Peter Maydell
Subject: Re: [Qemu-devel] [PATCH 3/4] Refactoring MonitorDef array
Date: Wed, 2 Oct 2013 00:21:09 +0900

On 2 October 2013 00:13, Richard Henderson <address@hidden> 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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]