On 02/09/2016 10:35 AM, Ján Tomko wrote: > On Tue, Feb 09, 2016 at 07:34:01AM -0500, John Ferlan wrote: >> Rather than have vnc be a variable within the if, promote it >> to the top, then adjust the code to use the error label to call >> virDomainGraphicsDefFree >> >> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx> >> --- >> src/qemu/qemu_command.c | 24 ++++++------------------ >> 1 file changed, 6 insertions(+), 18 deletions(-) >> >> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c >> index d7f19f3..7b5a36f 100644 >> --- a/src/qemu/qemu_command.c >> +++ b/src/qemu/qemu_command.c >> @@ -12999,6 +12999,7 @@ qemuParseCommandLine(virCapsPtr qemuCaps, >> int nvirtiodisk = 0; >> qemuDomainCmdlineDefPtr cmd = NULL; >> virDomainDiskDefPtr disk = NULL; >> + virDomainGraphicsDefPtr vnc = NULL; >> const char *ceph_args = qemuFindEnv(progenv, "CEPH_ARGS"); >> bool have_sdl = false; >> > > [...] > >> @@ -13991,6 +13978,7 @@ qemuParseCommandLine(virCapsPtr qemuCaps, >> return def; >> >> error: >> + virDomainGraphicsDefFree(vnc); >> virDomainDiskDefFree(disk); >> qemuDomainCmdlineDefFree(cmd); >> virDomainDefFree(def); > > The Free call is almost a thousand lines after the declaration. > This is a great opportunity to split out the VNC parsing into > a separate function, especially since it seems that its only input > is a const char* pointing to the command line option and the only > output is one virDomainGraphicsDef structure. > Doubly nice: move all this code to its own file like qemu_command_parse.c . AFAICT it has very little overlap with the rest of the (quite large) qemu_command.c - Cole -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list