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

[Daniel P. Berrange]

On Mon, Feb 09, 2009 at 03:39:15PM +0100, Jim Meyering wrote:
> "Daniel P. Berrange" <berrange at redhat.com> wrote:
> 
> > 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 ...
> 
> Can't do that.  The logDir member is declared like this:
> 
>   qemud/qemud.h:    char logDir[PATH_MAX];

No, I meant change that to just  'char *logDir' since pre-declaring
it PATH_MAX isn't really helping to simplify the code at all.

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