diff mbox

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

Message ID 1260824004-2941-5-git-send-email-lcapitulino@redhat.com
State New
Headers show

Commit Message

Luiz Capitulino Dec. 14, 2009, 8:53 p.m. UTC
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(-)

Comments

Markus Armbruster Dec. 15, 2009, 9:50 a.m. UTC | #1
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. UTC | #2
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. UTC | #3
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. UTC | #4
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. UTC | #5
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
diff mbox

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);