Re: [libvirt PATCH] All pointers to virXMLPropString() use g_autofree.

On 7/8/20 4:19 PM, Nicolas Brignone wrote: > All pointers to virXMLPropString() use g_autofree. I changed the summary line like this, to be more precise: conf: use g_autofree for all pointers to virXMLPropString() in device_conf.c > All modified functions are similar, in all cases "cleanup" label is removed, > along with all the "goto" calls. I've been advised in the recent past to put the g_autofree additions and corresponding removals of free functions into one patch, then do the removal of the extra labels (in favor of directly returning) in a separate patch to make it easier to hand-verify / review. Here's a couple recent examples: https://www.redhat.com/archives/libvir-list/2020-July/msg00317.html In your case the changes are few enough that I'm okay with it a single patch, except... > > Signed-off-by: Nicolas Brignone <nmbrignone(a)gmail.com&gt; > --- > src/conf/device_conf.c | 183 +++++++++++++---------------------------- > 1 file changed, 56 insertions(+), 127 deletions(-) > > diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c > index 7d48a3f..9fa6141 100644 > --- a/src/conf/device_conf.c > +++ b/src/conf/device_conf.c > @@ -208,45 +208,43 @@ int > virPCIDeviceAddressParseXML(xmlNodePtr node, > virPCIDeviceAddressPtr addr) > { > - char *domain, *slot, *bus, *function, *multi; > xmlNodePtr cur; > xmlNodePtr zpci = NULL; > - int ret = -1; > memset(addr, 0, sizeof(*addr)); > - domain = virXMLPropString(node, "domain"); > - bus = virXMLPropString(node, "bus"); > - slot = virXMLPropString(node, "slot"); > - function = virXMLPropString(node, "function"); > - multi = virXMLPropString(node, "multifunction"); > + g_autofree char *domain = virXMLPropString(node, "domain"); > + g_autofree char *bus = virXMLPropString(node, "bus"); > + g_autofree char *slot = virXMLPropString(node, "slot"); > + g_autofree char *function = virXMLPropString(node, "function"); > + g_autofree char *multi = virXMLPropString(node, "multifunction"); ... you've modified it so that local variables are being declared *below* a line of executable code rather than at the top of the block. Although I don't see anything in the coding style document (https://libvirt.org/coding-style.html) about it, and it may just be leftover from a time when we supported a compiler that required all declarations to be at top of scope, I think pretty much all of libvirts code declares all local variables at the top of the scope.