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 <[email protected]>
regardless of whether or not you decide to implement my suggestion.
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/libvir-list
