Re: [libvirt] [PATCH 01/15] util: Introduce VIR_DEFINE_AUTOPTR_FUNC for virStorageAuthDef

On 2/7/19 4:10 AM, Erik Skultety wrote: > On Wed, Feb 06, 2019 at 08:41:33AM -0500, John Ferlan wrote: >> Let's make use of the auto __cleanup capabilities cleaning up any >> now unnecessary goto paths. For virStorageAuthDefCopy use authdef >> and ret as consistently as similar other code. >> >> Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; >> --- >> src/conf/domain_conf.c | 29 +++++++++++------------------ >> src/conf/storage_conf.c | 6 ++---- >> src/qemu/qemu_parse_command.c | 6 ++---- >> src/util/virstoragefile.c | 33 ++++++++++++++------------------- >> src/util/virstoragefile.h | 1 + >> 5 files changed, 30 insertions(+), 45 deletions(-) >> >> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c >> index 1fc4c8a35a..5699a60549 100644 >> --- a/src/conf/domain_conf.c >> +++ b/src/conf/domain_conf.c >> @@ -7578,10 +7578,9 @@ virDomainHostdevSubsysSCSIiSCSIDefParseXML(xmlNodePtr sourcenode, >> virDomainHostdevSubsysSCSIPtr def, >> xmlXPathContextPtr ctxt) >> { >> - int ret = -1; >> int auth_secret_usage = -1; >> xmlNodePtr cur; >> - virStorageAuthDefPtr authdef = NULL; >> + VIR_AUTOPTR(virStorageAuthDef) authdef = NULL; > > For better readability I prefer declaring VIR_AUTO variables at the end of the > declare block (multiple occurrences throughout the patch)... > > ... > I don't mind moving them. I generally just try to keep the usages together. >> + VIR_STEAL_PTR(iscsisrc->src->auth, authdef); > > ^Unrelated change, there should be a trivial patch preceding this one taking > care of the VIR_STEAL_PTR replacements. > > ... You'll find this sprinkled throughout - generating separate patches could explode the series into perhaps 30+ patches which I doubt anyone would jump at the chance to review. I can separate before pushing and I assume by extracting it/them I can just add your R-by too...