Re: [libvirt] [PATCH v3 07/48] remote: conditionalize socket names in libvirtd daemon
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(a)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