From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51954) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b7gku-0006OR-K8 for qemu-devel@nongnu.org; Tue, 31 May 2016 06:18:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b7gkq-0001Bg-B2 for qemu-devel@nongnu.org; Tue, 31 May 2016 06:18:35 -0400 Received: from mga09.intel.com ([134.134.136.24]:56616) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b7gkq-0001Bc-4B for qemu-devel@nongnu.org; Tue, 31 May 2016 06:18:32 -0400 Message-ID: <1464688183.11816.11.camel@vmm.sh.intel.com> From: Robert Hu Reply-To: robert.hu@intel.com Date: Tue, 31 May 2016 17:49:43 +0800 In-Reply-To: <878tyq7jin.fsf@dusky.pond.sub.org> References: <1464676956-8460-1-git-send-email-robert.hu@intel.com> <1464676956-8460-3-git-send-email-robert.hu@intel.com> <878tyq7jin.fsf@dusky.pond.sub.org> Content-Type: text/plain; charset="ISO-8859-15" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/2] Explicitly print out default vnc option in use List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: Robert Ho , qemu-devel@nongnu.org, pbonzini@redhat.com On Tue, 2016-05-31 at 09:51 +0200, Markus Armbruster wrote: > Robert Ho writes: > > > If no display option defined in QEMU command line, and SDL is not > > available, then it by default uses '-vnc localhost:0,to=99,id=default'. > > This patch simply print out the default option parameters out, so that > > user is aware of that. > > > > Signed-off-by: Robert Ho > > --- > > vl.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/vl.c b/vl.c > > index 18d1423..8617a68 100644 > > --- a/vl.c > > +++ b/vl.c > > @@ -4213,7 +4213,10 @@ int main(int argc, char **argv, char **envp) > > #elif defined(CONFIG_COCOA) > > display_type = DT_COCOA; > > #elif defined(CONFIG_VNC) > > - vnc_parse("localhost:0,to=99,id=default", &error_abort); > > + #define DEFAULT_VNC_DISPLAY_OPTION "localhost:0,to=99,id=default" > > Preprocessor directives shouldn't be indented. > > Also tab damage. Please use scripts/checkpatch.pl to check your patches. Thanks Markus for your review. Firstly apologize if you received multiple copies of this patch. I'm still struggling with my egress SMTP setting. I've no idea how many copies you received:( but glad now see your reply. Yes, sorry about haven't checked the patch with the auxiliary scripts. I didn't know that. Thanks for pointing out. I'm new here, will learn these upstream convention ASAP. > > > + vnc_parse(DEFAULT_VNC_DISPLAY_OPTION, &error_abort); > > + printf("No display option defined, using '-vnc %s' by default \ > > +\n", DEFAULT_VNC_DISPLAY_OPTION); > > show_vnc_port = 1; > > #else > > display_type = DT_NONE; > > I don't like this. Programs should be quiet unless they got something > important to say. Can't see why this particular default is more > important than all the other defaults we don't print. > > The default could be documented in output of --help. Actually my thought was this is not using the default value and implicitly. The default of 'to' is 0, while in this case (when no display option defined and SDL not configured in), it implicitly uses non-default value '99'. Therefore I thought it shall be explicitly print out so that user would be aware of what was chosen on behalf of him; like the final print of 'VNC server running on '::1;5900''.