On Sat, Jul 28, 2018 at 11:31:17PM +0530, Sukrit Bhatnagar wrote:
> By making use of GNU C's cleanup attribute handled by the
> VIR_AUTOPTR macro for declaring aggregate pointer variables,
> majority of the calls to *Free functions can be dropped, which
> in turn leads to getting rid of most of our cleanup sections.
>
> Signed-off-by: Sukrit Bhatnagar <skrtbht...@gmail.com>
> ---
>  src/util/viriscsi.c | 68 
> +++++++++++++++++------------------------------------
>  1 file changed, 22 insertions(+), 46 deletions(-)
>
> diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c
> index 48f4106..81404f8 100644
> --- a/src/util/viriscsi.c
> +++ b/src/util/viriscsi.c
> @@ -91,7 +91,7 @@ virISCSIGetSession(const char *devpath,
>      VIR_AUTOFREE(char *) error = NULL;
>      int exitstatus = 0;
>
> -    virCommandPtr cmd = virCommandNewArgList(ISCSIADM, "--mode",
> +    VIR_AUTOPTR(virCommand) cmd = virCommandNewArgList(ISCSIADM, "--mode",
>                                               "session", NULL);

There are a few code misalignment issues like ^this. I'll fix them locally.

...

> @@ -576,9 +556,8 @@ virISCSINodeUpdate(const char *portal,
>                     const char *name,
>                     const char *value)
>  {
> -    virCommandPtr cmd = NULL;
> +    VIR_AUTOPTR(virCommand) cmd = NULL;

One tiny nitpick I realized only now is that we should probably start being
consistent in what order we declare the autoclean variables, IOW I think we
should group all the autoclean variables together so visually you can
immediately tell which variables are handled automatically and which aren't. It
was all fine across this file except for ^this single occurrence, I'll fix
that.

Reviewed-by: Erik Skultety <eskul...@redhat.com>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to