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/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;
-- 
2.38.1

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?

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!

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
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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!