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

Erik Skultety eskultet at redhat.com
Thu Feb 7 12:15:07 UTC 2019


On Thu, Feb 07, 2019 at 07:12:08AM -0500, John Ferlan wrote:
>
>
> 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 at redhat.com>
> >> ---
> >>  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...

That's why I mentioned trivial, I don't need to review those, as long as you
split them before pushing, I'm fine with that and the Rb applies, sorry for not
being clearer about that.

Erik




More information about the libvir-list mailing list