From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41848) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UzXZc-0007u4-Tb for qemu-devel@nongnu.org; Wed, 17 Jul 2013 15:39:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UzXZb-000221-1f for qemu-devel@nongnu.org; Wed, 17 Jul 2013 15:39:40 -0400 Received: from mx1.redhat.com ([209.132.183.28]:27157) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UzXV3-0000cY-7C for qemu-devel@nongnu.org; Wed, 17 Jul 2013 15:34:57 -0400 Date: Wed, 17 Jul 2013 15:34:51 -0400 From: Luiz Capitulino Message-ID: <20130717153451.1e2a2639@redhat.com> In-Reply-To: <1373512429-17865-6-git-send-email-xiawenc@linux.vnet.ibm.com> References: <1373512429-17865-1-git-send-email-xiawenc@linux.vnet.ibm.com> <1373512429-17865-6-git-send-email-xiawenc@linux.vnet.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH V6 05/13] monitor: avoid use of global *cur_mon in readline_completion() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Wenchao Xia Cc: pbonzini@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com On Thu, 11 Jul 2013 11:13:41 +0800 Wenchao Xia 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 > --- > 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++) {