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

[Andrea Bolognani]

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 at 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