Patchwork [4/5] monitor: Avoid readline functions in QMP

login
register
mail settings
Submitter Luiz Capitulino
Date Dec. 14, 2009, 8:53 p.m.
Message ID <1260824004-2941-5-git-send-email-lcapitulino@redhat.com>
Download mbox | patch
Permalink /patch/41137/
State New
Headers show

Comments

Luiz Capitulino - Dec. 14, 2009, 8:53 p.m.
The monitor_read_command() function is readline specific
and should only be used when readline is available.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
Markus Armbruster - Dec. 15, 2009, 9:50 a.m.
Luiz Capitulino <lcapitulino@redhat.com> writes:

> The monitor_read_command() function is readline specific
> and should only be used when readline is available.
>
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>

I figure this bug breaks password entry (VNC and block) on non-readline
monitors.  Suspect commit cde76ee1.
Markus Armbruster - Dec. 15, 2009, 9:54 a.m.
Markus Armbruster <armbru@redhat.com> writes:

> Luiz Capitulino <lcapitulino@redhat.com> writes:
>
>> The monitor_read_command() function is readline specific
>> and should only be used when readline is available.
>>
>> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
>
> I figure this bug breaks password entry (VNC and block) on non-readline
> monitors.  Suspect commit cde76ee1.

PS: Subject is wrong.  gdbserver_start() creates a monitor with
MONITOR_USE_READLINE off, so it's not just QMP.
Luiz Capitulino - Dec. 15, 2009, 12:08 p.m.
On Tue, 15 Dec 2009 10:54:23 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> Markus Armbruster <armbru@redhat.com> writes:
> 
> > Luiz Capitulino <lcapitulino@redhat.com> writes:
> >
> >> The monitor_read_command() function is readline specific
> >> and should only be used when readline is available.
> >>
> >> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> >
> > I figure this bug breaks password entry (VNC and block) on non-readline
> > monitors.  Suspect commit cde76ee1.

 I can only assume that it didn't matter until now.

> PS: Subject is wrong.  gdbserver_start() creates a monitor with
> MONITOR_USE_READLINE off, so it's not just QMP.

 Ditto.
Markus Armbruster - Dec. 15, 2009, 2:03 p.m.
Luiz Capitulino <lcapitulino@redhat.com> writes:

> On Tue, 15 Dec 2009 10:54:23 +0100
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> Markus Armbruster <armbru@redhat.com> writes:
>> 
>> > Luiz Capitulino <lcapitulino@redhat.com> writes:
>> >
>> >> The monitor_read_command() function is readline specific
>> >> and should only be used when readline is available.
>> >>
>> >> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
>> >
>> > I figure this bug breaks password entry (VNC and block) on non-readline
>> > monitors.  Suspect commit cde76ee1.
>
>  I can only assume that it didn't matter until now.
>
>> PS: Subject is wrong.  gdbserver_start() creates a monitor with
>> MONITOR_USE_READLINE off, so it's not just QMP.
>
>  Ditto.

Subject is wrong, because the commit avoids readline not only in QMP,
but whenever the monitor is configured not to use readline.  Yes, I'm
nitpicking :)
Jan Kiszka - Dec. 15, 2009, 2:17 p.m.
Luiz Capitulino wrote:
> On Tue, 15 Dec 2009 10:54:23 +0100
> Markus Armbruster <armbru@redhat.com> wrote:
> 
>> Markus Armbruster <armbru@redhat.com> writes:
>>
>>> Luiz Capitulino <lcapitulino@redhat.com> writes:
>>>
>>>> The monitor_read_command() function is readline specific
>>>> and should only be used when readline is available.
>>>>
>>>> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
>>> I figure this bug breaks password entry (VNC and block) on non-readline
>>> monitors.  Suspect commit cde76ee1.
> 
>  I can only assume that it didn't matter until now.
> 
>> PS: Subject is wrong.  gdbserver_start() creates a monitor with
>> MONITOR_USE_READLINE off, so it's not just QMP.
> 
>  Ditto.
> 

Right. monitor_read_password was never supposed to be used on monitors
that have not readline state and is reported as error by
monitor_read_password. So far the only monitor that was lacking such
state was the gdbserver, now we also have QMP. The latter handles this
separately, the former should still fail loudly when such commands are
issued.

Jan

Patch

diff --git a/monitor.c b/monitor.c
index 920ccff..b518cc4 100644
--- a/monitor.c
+++ b/monitor.c
@@ -140,6 +140,9 @@  static inline int monitor_ctrl_mode(const Monitor *mon)
 
 static void monitor_read_command(Monitor *mon, int show_prompt)
 {
+    if (!mon->rs)
+        return;
+
     readline_start(mon->rs, "(qemu) ", 0, monitor_command_cb, NULL);
     if (show_prompt)
         readline_show_prompt(mon->rs);