On Tue, 2019-07-30 at 12:24 +0200, Christophe de Dinechin wrote:
Daniel P. Berrangé writes:
+++ b/src/remote/remote_daemon.c @@ -221,19 +221,25 @@ daemonUnixSocketPaths(struct daemonConfig *config, 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 (virAsprintf(sockfile, "%s/run/libvirt/%s-sock", + LOCALSTATEDIR, SOCK_PREFIX) < 0 || + virAsprintf(sockfile, "%s/run/libvirt/%s-sock-ro", + LOCALSTATEDIR, SOCK_PREFIX) < 0 || + virAsprintf(sockfile, "%s/run/libvirt/%s-admin-sock", + LOCALSTATEDIR, SOCK_PREFIX) < 0)
Copy-paste error on sockfile variable name, use rosockfile and admsockfile.
Good catch, this definitely needs to be fixed before pushing.
Also, there is a memory leak if second or third fails, since the first one is never deallocated.
Consider adding a VIR_FREE for *sockfile, *rosockfile and *admsockfile in the cleanup section. Also, to make it real safe, consider adding a NULL-initialization for *sockfile, *rosockfile an d *admsockfile at the top of the function.
We can do this in a follow-up patch, especially since the issue is very much pre-exisisting. With the pastos fixed, Reviewed-by: Andrea Bolognani <abologna@redhat.com> [...]
@@ -902,12 +910,12 @@ daemonUsage(const char *argv0, bool privileged) fprintf(stderr, " %s:\n", _("Sockets"));
Localization of :
fprintf(stderr, " %s:\n", _("TLS"));
Localization of :
Both of these come from the previous patch. -- Andrea Bolognani / Red Hat / Virtualization