Am 12.06.2019 um 15:17 hat Markus Armbruster geschrieben:
> Kevin Wolf <kw...@redhat.com> writes:
>
> > Move HMP infrastructure from monitor/misc.c to monitor/hmp.c. This is
> > code that can be shared for all targets, so compile it only once.
> >
> > The amount of function and particularly extern variables in
> > monitor_int.h is probably a bit larger than it needs to be, but this way
> > no non-trivial code modifications are needed. The interfaces between HMP
> > and the monitor core can be cleaned up later.
> >
> > Signed-off-by: Kevin Wolf <kw...@redhat.com>
> > Reviewed-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
> > ---
> > include/monitor/monitor.h | 1 +
> > monitor/monitor_int.h | 31 +
> > monitor/hmp.c | 1387 +++++++++++++++++++++++++++++++++++++
> > monitor/misc.c | 1338 +----------------------------------
> > monitor/Makefile.objs | 2 +-
> > monitor/trace-events | 4 +-
> > 6 files changed, 1429 insertions(+), 1334 deletions(-)
> > create mode 100644 monitor/hmp.c
> >
> > diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
> > index 7bbab05320..8547529e49 100644
> > --- a/include/monitor/monitor.h
> > +++ b/include/monitor/monitor.h
> > @@ -22,6 +22,7 @@ bool monitor_cur_is_qmp(void);
> > void monitor_init_globals(void);
> > void monitor_init(Chardev *chr, int flags);
> > void monitor_init_qmp(Chardev *chr, int flags);
> > +void monitor_init_hmp(Chardev *chr, int flags);
> > void monitor_cleanup(void);
> >
> > int monitor_suspend(Monitor *mon);
> > diff --git a/monitor/monitor_int.h b/monitor/monitor_int.h
> > index 4aabee54e1..88eaed9c5c 100644
> > --- a/monitor/monitor_int.h
> > +++ b/monitor/monitor_int.h
> > @@ -27,6 +27,7 @@
> >
> > #include "qemu-common.h"
> > #include "monitor/monitor.h"
> > +#include "qemu/cutils.h"
> >
> > #include "qapi/qmp/qdict.h"
> > #include "qapi/qmp/json-parser.h"
> > @@ -154,6 +155,29 @@ static inline bool monitor_is_qmp(const Monitor *mon)
> > return (mon->flags & MONITOR_USE_CONTROL);
> > }
> >
> > +/**
> > + * Is @name in the '|' separated list of names @list?
> > + */
> > +static inline int compare_cmd(const char *name, const char *list)
> > +{
> > + const char *p, *pstart;
> > + int len;
> > + len = strlen(name);
> > + p = list;
> > + for (;;) {
> > + pstart = p;
> > + p = qemu_strchrnul(p, '|');
> > + if ((p - pstart) == len && !memcmp(pstart, name, len)) {
> > + return 1;
> > + }
> > + if (*p == '\0') {
> > + break;
> > + }
> > + p++;
> > + }
> > + return 0;
> > +}
> > +
>
> What's the justification for inline?
It seemed small enough, but maybe it isn't (it has also grown by two
lines after fixing the coding style). I can leave it in misc.c and just
make it public.
I'd just need a more specific name than compare_cmd() to make it public.
> > typedef QTAILQ_HEAD(MonitorList, Monitor) MonitorList;
> > extern IOThread *mon_iothread;
> > extern QEMUBH *qmp_dispatcher_bh;
> > @@ -162,6 +186,8 @@ extern QemuMutex monitor_lock;
> > extern MonitorList mon_list;
> > extern int mon_refcount;
> >
> > +extern mon_cmd_t mon_cmds[];
> > +
>
> Any particular reason for not moving this one to hmp.c, along with
> info_cmds? Question, not demand :)
Yes, it's not part of the core infrastructure, but contains commands
specific to the system emulator. If a tool were to use HMP, it would
have to provide its own command tables.
If we ever create a monitor/hmp-sysemu.c or something like it, this
would be a good place for the tables.
Kevin
