[libvirt] [PATCH] daemonUnixSocketPaths: Unify exit paths

[Erik Skultety]

On Tue, Jun 13, 2017 at 03:09:47PM +0200, Michal Privoznik wrote:
> Right now, there is a lot of exit points from the function.
> Depending on their position they need to copy the same free
> calls. This goes against our style where we usually have just one
> exit point from the function which also does the necessary free.
>
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  daemon/libvirtd.c | 35 ++++++++++++++++-------------------
>  1 file changed, 16 insertions(+), 19 deletions(-)
>
> diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
> index d17a694c9..db239f0d4 100644
> --- a/daemon/libvirtd.c
> +++ b/daemon/libvirtd.c
> @@ -262,50 +262,47 @@ daemonUnixSocketPaths(struct daemonConfig *config,
>                        char **rosockfile,
>                        char **admsockfile)
>  {
> +    int ret = -1;
> +    char *rundir = NULL;
> +
>      if (config->unix_sock_dir) {
>          if (virAsprintf(sockfile, "%s/libvirt-sock", config->unix_sock_dir) < 0)
> -            goto error;
> +            goto cleanup;
>
>          if (privileged) {
> -            if (virAsprintf(rosockfile, "%s/libvirt-sock-ro", config->unix_sock_dir) < 0)
> -                goto error;
> -            if (virAsprintf(admsockfile, "%s/libvirt-admin-sock", config->unix_sock_dir) < 0)
> -                goto error;
> +            if (virAsprintf(rosockfile, "%s/libvirt-sock-ro", config->unix_sock_dir) < 0 ||
> +                virAsprintf(admsockfile, "%s/libvirt-admin-sock", config->unix_sock_dir) < 0)
> +                goto cleanup;

So, we tend to wrap the lines at 80 chars, but we don't enforce it anywhere, we
just recommend it. Now, in this particular case, although over, I must admit
that it's easier to read, it's much more self-contained so to speak.

>          }
>      } else {
>          if (privileged) {
>              if (VIR_STRDUP(*sockfile, LOCALSTATEDIR "/run/libvirt/libvirt-sock") < 0 ||
>                  VIR_STRDUP(*rosockfile, LOCALSTATEDIR "/run/libvirt/libvirt-sock-ro") < 0 ||
>                  VIR_STRDUP(*admsockfile, LOCALSTATEDIR "/run/libvirt/libvirt-admin-sock") < 0)

Same here.

> -                goto error;
> +                goto cleanup;
>          } else {
> -            char *rundir = NULL;
>              mode_t old_umask;
>
>              if (!(rundir = virGetUserRuntimeDirectory()))
> -                goto error;
> +                goto cleanup;
>
>              old_umask = umask(077);
>              if (virFileMakePath(rundir) < 0) {
>                  umask(old_umask);
> -                VIR_FREE(rundir);
> -                goto error;
> +                goto cleanup;
>              }
>              umask(old_umask);
>
>              if (virAsprintf(sockfile, "%s/libvirt-sock", rundir) < 0 ||
> -                virAsprintf(admsockfile, "%s/libvirt-admin-sock", rundir) < 0) {
> -                VIR_FREE(rundir);
> -                goto error;
> -            }
> -
> -            VIR_FREE(rundir);
> +                virAsprintf(admsockfile, "%s/libvirt-admin-sock", rundir) < 0)
> +                goto cleanup;
>          }
>      }
> -    return 0;
>
> - error:
> -    return -1;
> +    ret = 0;
> + cleanup:
> +    VIR_FREE(rundir);
> +    return ret;
>  }
>

So ACK, even with the longer lines.

Erik

>
> --
> 2.13.0
>