On Wed, Aug 21, 2013 at 11:15:39AM +0200, Martin Kletzander wrote: > Commit abfff210 changed the order of vshParseArgv() and vshInit() in > order to make fix debugging of parameter parsing. However, vshInit() > did a vshReconnect() even though ctl->name wasn't set according to the > '-c' parameter yet. In order to keep both issues fixed, I've split > the vshInit() into vshInitDebug() and vshInit(). > > One simple memleak of ctl->name is fixed as a part of this patch, > since it is related to the issue it's fixing. > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=999323 > Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx> > --- > tools/virsh.c | 28 +++++++++++++++++----------- > 1 file changed, 17 insertions(+), 11 deletions(-) This looks like something we can usefully test for. eg create a test suite that does something like this URI1=test:///$(top_srcdir)/examples/test/testnode.xml URI2=test:///$(top_srcdir)/examples/test/testnodeinline.xml export LIBVIRT_DEFAULT_URI=$URI1 GOTURI=`virsh -c $URI2 uri` if $GOTURI == $URI2 ....pass... else ...fail... fi > > diff --git a/tools/virsh.c b/tools/virsh.c > index 15f529e..2ea44a6 100644 > --- a/tools/virsh.c > +++ b/tools/virsh.c > @@ -2294,16 +2294,13 @@ vshEventLoop(void *opaque) > > > /* > - * Initialize connection. > + * Initialize debug settings. > */ > -static bool > -vshInit(vshControl *ctl) > +static void > +vshInitDebug(vshControl *ctl) > { > char *debugEnv; > > - if (ctl->conn) > - return false; > - > if (ctl->debug == VSH_DEBUG_DEFAULT) { > /* log level not set from commandline, check env variable */ > debugEnv = getenv("VIRSH_DEBUG"); > @@ -2328,6 +2325,16 @@ vshInit(vshControl *ctl) > } > > vshOpenLogFile(ctl); > +} > + > +/* > + * Initialize connection. > + */ > +static bool > +vshInit(vshControl *ctl) > +{ > + if (ctl->conn) > + return false; > > /* set up the library error handler */ > virSetErrorFunc(NULL, virshErrorHandler); > @@ -3017,6 +3024,7 @@ vshParseArgv(vshControl *ctl, int argc, char **argv) > ctl->timing = true; > break; > case 'c': > + VIR_FREE(ctl->name); > ctl->name = vshStrdup(ctl, optarg); > break; > case 'v': > @@ -3192,12 +3200,10 @@ main(int argc, char **argv) > ctl->name = vshStrdup(ctl, defaultConn); > } > > - if (!vshInit(ctl)) { > - vshDeinit(ctl); > - exit(EXIT_FAILURE); > - } > + vshInitDebug(ctl); > > - if (!vshParseArgv(ctl, argc, argv)) { > + if (!vshParseArgv(ctl, argc, argv) || > + !vshInit(ctl)) { > vshDeinit(ctl); > exit(EXIT_FAILURE); > } ACK -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list