[V6,05/13] monitor: avoid use of global *cur_mon in readline_completion()

Now all completion functions do not use *cur_mon any more, instead
they use rs->mon. In short, structure ReadLineState decide where
the complete action would be taken now.

Tested with the case that qemu have two telnet monitors, auto
completion function works normal.

Take a better look at monitor_init(), it calls readline_init() which
initialize mon->rs, result is mon->rs->mon == mon. Then it calls
qemu_chr_add_handlers(), which make monitor_read() function
take *mon as its opaque. Later, when user input, monitor_read()
is called, where cur_mon is set to *mon by "cur_mon = opaque".
If qemu's monitors run in one thread, then later in
readline_handle_byte() and readline_comletion(), cur_mon is actually
equal to rs->mon, in another word it points to the actuall monitor
instance, so it is safe to replace *cur_mon in those functions.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 readline.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

On Thu, 11 Jul 2013 11:13:41 +0800
Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:

> Now all completion functions do not use *cur_mon any more, instead
> they use rs->mon. In short, structure ReadLineState decide where
> the complete action would be taken now.
> 
> Tested with the case that qemu have two telnet monitors, auto
> completion function works normal.
> 
> Take a better look at monitor_init(), it calls readline_init() which
> initialize mon->rs, result is mon->rs->mon == mon. Then it calls
> qemu_chr_add_handlers(), which make monitor_read() function
> take *mon as its opaque. Later, when user input, monitor_read()
> is called, where cur_mon is set to *mon by "cur_mon = opaque".
> If qemu's monitors run in one thread, then later in
> readline_handle_byte() and readline_comletion(), cur_mon is actually
> equal to rs->mon, in another word it points to the actuall monitor
> instance, so it is safe to replace *cur_mon in those functions.

I find this paragraph a bit confusing and not exactly related to
this patch. I know I asked you to explain why dropping cur_mon usage
shouldn't brake things, but looks like you don't have a good place
to have that info.

> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  readline.c |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
> 
> diff --git a/readline.c b/readline.c
> index c91b324..abf27dd 100644
> --- a/readline.c
> +++ b/readline.c
> @@ -276,7 +276,6 @@ void readline_set_completion_index(ReadLineState *rs, int index)
>  
>  static void readline_completion(ReadLineState *rs)
>  {
> -    Monitor *mon = cur_mon;
>      int len, i, j, max_width, nb_cols, max_prefix;
>      char *cmdline;
>  
> @@ -300,7 +299,7 @@ static void readline_completion(ReadLineState *rs)
>          if (len > 0 && rs->completions[0][len - 1] != '/')
>              readline_insert_char(rs, ' ');
>      } else {
> -        monitor_printf(mon, "\n");
> +        monitor_printf(rs->mon, "\n");
>          max_width = 0;
>          max_prefix = 0;	
>          for(i = 0; i < rs->nb_completions; i++) {

于 2013-7-18 3:34, Luiz Capitulino 写道:
> On Thu, 11 Jul 2013 11:13:41 +0800
> Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>
>> Now all completion functions do not use *cur_mon any more, instead
>> they use rs->mon. In short, structure ReadLineState decide where
>> the complete action would be taken now.
>>
>> Tested with the case that qemu have two telnet monitors, auto
>> completion function works normal.
>>
>> Take a better look at monitor_init(), it calls readline_init() which
>> initialize mon->rs, result is mon->rs->mon == mon. Then it calls
>> qemu_chr_add_handlers(), which make monitor_read() function
>> take *mon as its opaque. Later, when user input, monitor_read()
>> is called, where cur_mon is set to *mon by "cur_mon = opaque".
>> If qemu's monitors run in one thread, then later in
>> readline_handle_byte() and readline_comletion(), cur_mon is actually
>> equal to rs->mon, in another word it points to the actuall monitor
>> instance, so it is safe to replace *cur_mon in those functions.
>
> I find this paragraph a bit confusing and not exactly related to
> this patch. I know I asked you to explain why dropping cur_mon usage
> shouldn't brake things, but looks like you don't have a good place
> to have that info.
>
>>
   How about refine the message as:
"Now usage of cur_mon is dropped in readline_completion(), it is safe
because:
.....
"
?

On Thu, 18 Jul 2013 09:55:51 +0800
Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:

> 于 2013-7-18 3:34, Luiz Capitulino 写道:
> > On Thu, 11 Jul 2013 11:13:41 +0800
> > Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
> >
> >> Now all completion functions do not use *cur_mon any more, instead
> >> they use rs->mon. In short, structure ReadLineState decide where
> >> the complete action would be taken now.
> >>
> >> Tested with the case that qemu have two telnet monitors, auto
> >> completion function works normal.
> >>
> >> Take a better look at monitor_init(), it calls readline_init() which
> >> initialize mon->rs, result is mon->rs->mon == mon. Then it calls
> >> qemu_chr_add_handlers(), which make monitor_read() function
> >> take *mon as its opaque. Later, when user input, monitor_read()
> >> is called, where cur_mon is set to *mon by "cur_mon = opaque".
> >> If qemu's monitors run in one thread, then later in
> >> readline_handle_byte() and readline_comletion(), cur_mon is actually
> >> equal to rs->mon, in another word it points to the actuall monitor
> >> instance, so it is safe to replace *cur_mon in those functions.
> >
> > I find this paragraph a bit confusing and not exactly related to
> > this patch. I know I asked you to explain why dropping cur_mon usage
> > shouldn't brake things, but looks like you don't have a good place
> > to have that info.
> >
> >>
>    How about refine the message as:
> "Now usage of cur_mon is dropped in readline_completion(), it is safe
> because:
> .....
> "
> ?

I think you have to just describe what this commit does.