|
From: | Pavel Butsykin |
Subject: | Re: [Qemu-devel] [PATCH 2/3] monitor: remove target-specific code from monitor.c |
Date: | Fri, 28 Aug 2015 12:21:45 +0300 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 |
On 27.08.2015 20:31, Peter Maydell
wrote:
Yes, this is a good way if we make the interface: const MonitorDef *target_monitor_defs(void);On 12 August 2015 at 12:50, Denis V. Lunev <address@hidden> wrote:From: Pavel Butsykin <address@hidden> Move target-specific code out of /monitor.c to /target-*/monitor.c, this will avoid code cluttering and using random ifdeffery. The solution is quite simple, but solves the issue of the separation of target-specific code from monitor Signed-off-by: Pavel Butsykin <address@hidden> Signed-off-by: Denis V. Lunev <address@hidden> CC: Luiz Capitulino <address@hidden> CC: Paolo Bonzini <address@hidden> CC: Peter Maydell <address@hidden> --- include/monitor/monitor-common.h | 43 ++ monitor.c | 854 +-------------------------------------- target-i386/Makefile.objs | 2 +- target-i386/monitor.c | 489 ++++++++++++++++++++++ target-ppc/Makefile.objs | 2 +- target-ppc/monitor.c | 250 ++++++++++++ target-sh4/Makefile.objs | 1 + target-sh4/monitor.c | 52 +++ target-sparc/Makefile.objs | 2 +- target-sparc/monitor.c | 153 +++++++ target-xtensa/Makefile.objs | 1 + target-xtensa/monitor.c | 34 ++ 12 files changed, 1032 insertions(+), 851 deletions(-) create mode 100644 include/monitor/monitor-common.h create mode 100644 target-i386/monitor.c create mode 100644 target-ppc/monitor.c create mode 100644 target-sh4/monitor.c create mode 100644 target-sparc/monitor.c create mode 100644 target-xtensa/monitor.c+#if defined(TARGET_SPARC) || defined(TARGET_PPC) || defined(TARGET_I386) +extern const MonitorDef monitor_defs[]; +#else +const MonitorDef monitor_defs[] = {}; #endifSo, rather than having to have a list of which targets provide a monitor_defs[], I would suggest that we make the API implemented by the target be a function, like: const MonitorDef *target_monitor_defs(void); (which just returns a pointer to a static const array in the target-*/monitor.c file). Then you can add a file stubs/target-monitor-defs.c which provides the "stub" version of this function (just returns a pointer to the no-commands array). The link process will arrange that the stub version is pulled in for any target that doesn't provide its own implementation of the function. Other than that, I suspect we can improve the separation out of target-specific things, but this is a good improvement and it'll be easier to do the rest as incremental fixes on top of this later. thanks -- PMM But we can't include the 'monitor/monitor-common.h' to stubs/target-monitor-defs.c, because there is a dependency with a target-specific headers( such as cpu.h:CPUArchState, cpu-defs.h:target_long). Make a copy of the struct MonitorDef not a good way because we can miss the change of copied MonitorDef in stubs/target-monitor-defs.c and this will result in an bug. Can this be solved somehow else? |
[Prev in Thread] | Current Thread | [Next in Thread] |