[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] Re: monitor: check for readline in monitor_event()
From: |
Luiz Capitulino |
Subject: |
Re: [Qemu-devel] Re: monitor: check for readline in monitor_event() |
Date: |
Tue, 16 Jun 2009 13:04:30 -0300 |
On Tue, 16 Jun 2009 17:46:05 +0200
Jan Kiszka <address@hidden> wrote:
> Luiz Capitulino wrote:
> > On Tue, 16 Jun 2009 00:42:26 +0200
> > Jan Kiszka <address@hidden> wrote:
> >
> >> Luiz Capitulino wrote:
> >>> The call of readline_show_prompt() in CHR_EVENT_RESET's body will
> >>> trig a segfault if readline is not being used, because 'mon->rs'
> >>> will be NULL.
> >>>
> >>> This fixes the problem by adding the proper check.
> >>>
> >>> I've trigged this while playing with an off-tree code that
> >>> disables readline support, I'm not sure whether in-tree code can
> >>> trig this.
> >>>
> >>> Signed-off-by: Luiz Capitulino <address@hidden>
> >>> ---
> >>> monitor.c | 2 +-
> >>> 1 files changed, 1 insertions(+), 1 deletions(-)
> >>>
> >>> diff --git a/monitor.c b/monitor.c
> >>> index 6b45f6c..787101d 100644
> >>> --- a/monitor.c
> >>> +++ b/monitor.c
> >>> @@ -3002,7 +3002,7 @@ static void monitor_event(void *opaque, int
> >>> event) case CHR_EVENT_RESET:
> >>> monitor_printf(mon, "QEMU %s monitor - type 'help' for
> >>> more " "information\n", QEMU_VERSION);
> >>> - if (mon->chr->focus == 0)
> >>> + if (mon->rs && mon->chr->focus == 0)
> >>> readline_show_prompt(mon->rs);
> >>> break;
> >>> }
> >> In-tree code is not affected as no monitor user will call
> >> qemu_chr_reset for the associated char device if there is no
> >> readline active as well. Yeah, secret de-facto rule.
> >
> > Ok.
> >
> >> The patch is not incorrect, but I would like to understand the
> >> out-of-tree use case behind it. So you do want the info line
> >> printed, but provide your own readline processor? What kind of
> >> terminal is this?
> >
> > I'm working on a machine-friendly monitor for QEMU, which disables
> > readline.
>
> Again, I bet that your monitor will not be interested in the prompt
> header. So I think we should rather exclude the whole CHR_EVENT_RESET
> path for the !mon->rs case.
It has a greeting message, although my real question is whether it's
going to support the mux part or not. If it's not, then I can have
my own reset handler.
Ouch, I said I didn't want to get into details. :)
> > I didn't want to get into the details now, it will be better to
> > discuss the prototype I'm writing, should be finished soon.
>
> Looking forward!
Great, will appreciate your feedback.