[libvirt] [PATCH 07/41] remote: conditionalize socket names in libvirtd daemon

[Andrea Bolognani]

On Tue, 2019-07-23 at 17:02 +0100, Daniel P. Berrangé wrote:
[...]
>      if (config->unix_sock_dir) {
> -        if (virAsprintf(sockfile, "%s/libvirt-sock", config->unix_sock_dir) < 0)
> +        if (virAsprintf(sockfile, "%s/" SOCK_PREFIX "-sock", config->unix_sock_dir) < 0)
>              goto cleanup;

Since you're using virAsprintf() already, I'd write this as

  virAsprintf(sockfile, "%s/%s-sock", SOCK_PREFIX, config->unix_sock_dir)

instead of using static string concatenation: it looks a bit cleaner
in my opinion.

[...]
>          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)
> +            if (VIR_STRDUP(*sockfile, LOCALSTATEDIR "/run/libvirt/" SOCK_PREFIX "-sock") < 0 ||
> +                VIR_STRDUP(*rosockfile, LOCALSTATEDIR "/run/libvirt/" SOCK_PREFIX "-sock-ro") < 0 ||
> +                VIR_STRDUP(*admsockfile, LOCALSTATEDIR "/run/libvirt/" SOCK_PREFIX "-admin-sock") < 0)
>                  goto cleanup;

These are not using virAsprintf() but could easily be converted.

[...]
>      fprintf(stderr, "    %s:\n", _("Sockets"));
> -    fprintf(stderr, "      %s\n",
> -            privileged ? LOCALSTATEDIR "/run/libvirt/libvirt-sock" :
> -            "$XDG_RUNTIME_DIR/libvirt/libvirt-sock");
> +    fprintf(stderr, "      %s/libvirt/" SOCK_PREFIX "-sock\n",
> +            privileged ? LOCALSTATEDIR "/run" :
> +            "$XDG_RUNTIME_DIR");

All fprintf() calls could be converted as well, except for this one
where the conversion would require adding one extra step and thus is
probably not worth it.


While I think following this proposal would result in slightly
cleaner code, functionally both approaches are identicaly so

  Reviewed-by: Andrea Bolognani <abologna at redhat.com>

regardless of whether or not you decide to implement my suggestion.

-- 
Andrea Bolognani / Red Hat / Virtualization