[libvirt] Re: [PATCH] libvirtd: new config-file option: unix_sock_dir [was Re: adding tests....

[Daniel P. Berrange]

On Mon, Feb 09, 2009 at 02:01:19PM +0100, Jim Meyering wrote:
> However, first things first:
> 
> Here's a patch that adds two blocks, neither pretty,
> but with less duplication than the 3rd alternative,
> which duplicates both the snprintf and the result comparison.
> (of course, I'll use only one of them)
> 
> BTW, this also eliminates the uses of PATH_MAX that were
> vestiges of a messy rebase.  Now we test against maxlen.
> 
> Of these two, I prefer the latter (slightly less duplication).
> Do you care?

I don't particular like either option - too much line noise in there.
Two thoughts - printf is redundant in the first place, since the compiler
will happily concatenate 2 static strings, so we can just strdup(). In
the second case, we could asprintf instead, and so have something like

  if (uid == SYSTEM_UID)
     server->logDir = strdup(LOCAL_STATE_DIR "/log/libvirt")
  else
     virAsprintf(&server->logDir, "%s/.libvirt/log", dir_prefix))

  if (!server->logDir)
     ... oom handling ...

> 
> diff --git a/qemud/qemud.c b/qemud/qemud.c
> index f8c3c97..ddcb6ff 100644
> --- a/qemud/qemud.c
> +++ b/qemud/qemud.c
> @@ -766,8 +766,16 @@ static int qemudInitPaths(struct qemud_server *server,
>              goto snprintf_error;
>      }
> 
> -    if (snprintf(server->logDir, PATH_MAX, "%s/.libvirt/log",
> -                 dir_prefix) >= PATH_MAX)
> +    if ((uid == SYSTEM_UID
> +         ? snprintf(server->logDir, maxlen, "%s/log/libvirt", LOCAL_STATE_DIR)
> +         : snprintf(server->logDir, maxlen, "%s/.libvirt/log", dir_prefix))
> +        >= maxlen)
> +        goto snprintf_error;
> +
> +    if (snprintf(server->logDir, maxlen,
> +                 (uid == SYSTEM_UID ? "%s/log/libvirt" : "%s/.libvirt/log"),
> +                 (uid == SYSTEM_UID ? LOCAL_STATE_DIR : dir_prefix))
> +        >= maxlen)
>          goto snprintf_error;
> 


Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|