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@redhat.com> regardless of whether or not you decide to implement my suggestion. -- Andrea Bolognani / Red Hat / Virtualization