On Tue, 2016-05-31 at 13:17 +0200, Markus Armbruster wrote: > Robert Hu <robert...@vmm.sh.intel.com> writes: > > > On Tue, 2016-05-31 at 09:51 +0200, Markus Armbruster wrote: > >> Robert Ho <robert...@intel.com> 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 <robert...@intel.com> > >> > --- > >> > 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. > > No problem. All we expect from new contributors is making an effort to > get their patches right. Actually getting them 100% right from the > start isn't really in the cards :)
Thank You! Sorry for late following up; for I just part-time do this. I'm too busy on work these days. > > >> > + 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''. > > The default depends on configuration options. Ideally, --help output > would show the defaults for this build's configuration. I don't see a './configure' option related to this '-vnc to' param. Is there any? '--help', you mean 'qemu-system_x86-64 --help'? or './configure --help'? > > If we decide users need more information than the current "VNC server > running on" line, perhaps it should be included right in that line.