From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55394) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZV17T-0003vp-JX for qemu-devel@nongnu.org; Thu, 27 Aug 2015 13:37:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZV17Q-0006up-Cd for qemu-devel@nongnu.org; Thu, 27 Aug 2015 13:37:47 -0400 Received: from mx2.parallels.com ([199.115.105.18]:36022) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZV17Q-0006uj-70 for qemu-devel@nongnu.org; Thu, 27 Aug 2015 13:37:44 -0400 References: <1439380232-20660-1-git-send-email-den@openvz.org> <1439380232-20660-3-git-send-email-den@openvz.org> From: "Denis V. Lunev" Message-ID: <55DF4ADB.6030203@openvz.org> Date: Thu, 27 Aug 2015 20:37:31 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/3] monitor: remove target-specific code from monitor.c List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Paolo Bonzini , Luiz Capitulino , QEMU Developers , Pavel Butsykin On 08/27/2015 08:31 PM, Peter Maydell wrote: > On 12 August 2015 at 12:50, Denis V. Lunev wrote: >> From: Pavel Butsykin >> >> 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 >> Signed-off-by: Denis V. Lunev >> CC: Luiz Capitulino >> CC: Paolo Bonzini >> CC: Peter Maydell >> --- >> 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[] = {}; >> #endif > So, 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 ok, sounds good to me Thank you for the suggestion