| Message ID | 20221220090645.2844881-17-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | ui: Move and clean up monitor command code | expand |
On Tue, Dec 20, 2022 at 10:06:43AM +0100, Markus Armbruster wrote: > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > include/monitor/hmp.h | 1 + > include/ui/console.h | 2 +- > monitor/misc.c | 1 - > ui/input.c | 5 +---- > ui/ui-hmp-cmds.c | 8 ++++++++ > 5 files changed, 11 insertions(+), 6 deletions(-) > > -void hmp_mouse_set(Monitor *mon, const QDict *qdict) > +void qemu_mouse_set(int index, Error **err) This is adding a Error parameter, nit s/err/errp/ > { > QemuInputHandlerState *s; > - int index = qdict_get_int(qdict, "index"); > int found = 0; > > QTAILQ_FOREACH(s, &handlers, node) { But not changing either error_report() call to error_setg(), so the errp is unused. With regards, Daniel
On 20/12/22 10:06, Markus Armbruster wrote: > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > include/monitor/hmp.h | 1 + > include/ui/console.h | 2 +- > monitor/misc.c | 1 - > ui/input.c | 5 +---- > ui/ui-hmp-cmds.c | 8 ++++++++ > 5 files changed, 11 insertions(+), 6 deletions(-) > diff --git a/include/ui/console.h b/include/ui/console.h > index e400ee9fa7..e7f375312c 100644 > --- a/include/ui/console.h > +++ b/include/ui/console.h > @@ -65,7 +65,7 @@ void qemu_remove_led_event_handler(QEMUPutLEDEntry *entry); > > void kbd_put_ledstate(int ledstate); > > -void hmp_mouse_set(Monitor *mon, const QDict *qdict); > +void qemu_mouse_set(int index, Error **err); Can we return a boolean to indicate eventual failure? > @@ -594,10 +592,9 @@ MouseInfoList *qmp_query_mice(Error **errp) > return mice_list; > } > > -void hmp_mouse_set(Monitor *mon, const QDict *qdict) > +void qemu_mouse_set(int index, Error **err) > { > QemuInputHandlerState *s; > - int index = qdict_get_int(qdict, "index"); > int found = 0; > > QTAILQ_FOREACH(s, &handlers, node) { However no failure is reported, errp isn't used... Do we really want to pass it along?
Daniel P. Berrangé <berrange@redhat.com> writes: > On Tue, Dec 20, 2022 at 10:06:43AM +0100, Markus Armbruster wrote: >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> --- >> include/monitor/hmp.h | 1 + >> include/ui/console.h | 2 +- >> monitor/misc.c | 1 - >> ui/input.c | 5 +---- >> ui/ui-hmp-cmds.c | 8 ++++++++ >> 5 files changed, 11 insertions(+), 6 deletions(-) > >> >> -void hmp_mouse_set(Monitor *mon, const QDict *qdict) >> +void qemu_mouse_set(int index, Error **err) > > This is adding a Error parameter, nit s/err/errp/ Yes. >> { >> QemuInputHandlerState *s; >> - int index = qdict_get_int(qdict, "index"); >> int found = 0; >> >> QTAILQ_FOREACH(s, &handlers, node) { > > But not changing either error_report() call to error_setg(), > so the errp is unused. I suspect I some distraction hit my flu-addled brain in just the right moment, and I forgot to flip to error_setg(). Will fix, thanks!
Philippe Mathieu-Daudé <philmd@linaro.org> writes: > On 20/12/22 10:06, Markus Armbruster wrote: >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> --- >> include/monitor/hmp.h | 1 + >> include/ui/console.h | 2 +- >> monitor/misc.c | 1 - >> ui/input.c | 5 +---- >> ui/ui-hmp-cmds.c | 8 ++++++++ >> 5 files changed, 11 insertions(+), 6 deletions(-) > > >> diff --git a/include/ui/console.h b/include/ui/console.h >> index e400ee9fa7..e7f375312c 100644 >> --- a/include/ui/console.h >> +++ b/include/ui/console.h >> @@ -65,7 +65,7 @@ void qemu_remove_led_event_handler(QEMUPutLEDEntry *entry); >> >> void kbd_put_ledstate(int ledstate); >> >> -void hmp_mouse_set(Monitor *mon, const QDict *qdict); >> +void qemu_mouse_set(int index, Error **err); > > Can we return a boolean to indicate eventual failure? The only caller isn't interested. But we could do it anyway to conform to the guidance in qapi/error.h. >> @@ -594,10 +592,9 @@ MouseInfoList *qmp_query_mice(Error **errp) >> return mice_list; >> } >> >> -void hmp_mouse_set(Monitor *mon, const QDict *qdict) >> +void qemu_mouse_set(int index, Error **err) >> { >> QemuInputHandlerState *s; >> - int index = qdict_get_int(qdict, "index"); >> int found = 0; >> >> QTAILQ_FOREACH(s, &handlers, node) { > > However no failure is reported, errp isn't used... Do we really > want to pass it along? Daniel spotted this, too. See my reply there. Thanks!
diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h index df89eac22a..8688769a27 100644 --- a/include/monitor/hmp.h +++ b/include/monitor/hmp.h @@ -88,6 +88,7 @@ void hmp_getfd(Monitor *mon, const QDict *qdict); void hmp_closefd(Monitor *mon, const QDict *qdict); void hmp_mouse_move(Monitor *mon, const QDict *qdict); void hmp_mouse_button(Monitor *mon, const QDict *qdict); +void hmp_mouse_set(Monitor *mon, const QDict *qdict); void hmp_sendkey(Monitor *mon, const QDict *qdict); void coroutine_fn hmp_screendump(Monitor *mon, const QDict *qdict); void hmp_chardev_add(Monitor *mon, const QDict *qdict); diff --git a/include/ui/console.h b/include/ui/console.h index e400ee9fa7..e7f375312c 100644 --- a/include/ui/console.h +++ b/include/ui/console.h @@ -65,7 +65,7 @@ void qemu_remove_led_event_handler(QEMUPutLEDEntry *entry); void kbd_put_ledstate(int ledstate); -void hmp_mouse_set(Monitor *mon, const QDict *qdict); +void qemu_mouse_set(int index, Error **err); /* keysym is a unicode code except for special keys (see QEMU_KEY_xxx constants) */ diff --git a/monitor/misc.c b/monitor/misc.c index 3d68940d28..50cb9f008b 100644 --- a/monitor/misc.c +++ b/monitor/misc.c @@ -33,7 +33,6 @@ #include "ui/qemu-spice.h" #include "qemu/config-file.h" #include "qemu/ctype.h" -#include "ui/console.h" #include "audio/audio.h" #include "disas/disas.h" #include "qemu/timer.h" diff --git a/ui/input.c b/ui/input.c index 8f4a87d1d7..7bece94e79 100644 --- a/ui/input.c +++ b/ui/input.c @@ -2,8 +2,6 @@ #include "sysemu/sysemu.h" #include "qapi/error.h" #include "qapi/qapi-commands-ui.h" -#include "qapi/qmp/qdict.h" -#include "qemu/error-report.h" #include "trace.h" #include "ui/input.h" #include "ui/console.h" @@ -594,10 +592,9 @@ MouseInfoList *qmp_query_mice(Error **errp) return mice_list; } -void hmp_mouse_set(Monitor *mon, const QDict *qdict) +void qemu_mouse_set(int index, Error **err) { QemuInputHandlerState *s; - int index = qdict_get_int(qdict, "index"); int found = 0; QTAILQ_FOREACH(s, &handlers, node) { diff --git a/ui/ui-hmp-cmds.c b/ui/ui-hmp-cmds.c index adea425d35..ad6a2a52e2 100644 --- a/ui/ui-hmp-cmds.c +++ b/ui/ui-hmp-cmds.c @@ -69,6 +69,14 @@ void hmp_mouse_button(Monitor *mon, const QDict *qdict) mouse_button_state = button_state; } +void hmp_mouse_set(Monitor *mon, const QDict *qdict) +{ + Error *err = NULL; + + qemu_mouse_set(qdict_get_int(qdict, "index"), &err); + hmp_handle_error(mon, err); +} + void hmp_info_mice(Monitor *mon, const QDict *qdict) { MouseInfoList *mice_list, *mouse;
Signed-off-by: Markus Armbruster <armbru@redhat.com> --- include/monitor/hmp.h | 1 + include/ui/console.h | 2 +- monitor/misc.c | 1 - ui/input.c | 5 +---- ui/ui-hmp-cmds.c | 8 ++++++++ 5 files changed, 11 insertions(+), 6 deletions(-)