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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
> ---
>  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)...

...

> +            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.

...

>
> diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
> index a98d5103fa..f1164dde9b 100644
> --- a/src/util/virstoragefile.h
> +++ b/src/util/virstoragefile.h
> @@ -543,4 +543,5 @@ void virStorageFileReportBrokenChain(int errcode,
>                                       virStorageSourcePtr src,
>                                       virStorageSourcePtr parent);
>
> +VIR_DEFINE_AUTOPTR_FUNC(virStorageAuthDef, virStorageAuthDefFree);

^This defines a static function, so the ';' is unnecessary, it doesn't hurt,
but you know, consistency ;). Also, keep the original newline below.

Reviewed-by: Erik Skultety <eskultet@xxxxxxxxxx>


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux