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

[Jim Meyering]

"Daniel P. Berrange" <berrange at redhat.com> wrote:

> 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

Um. checking snprintf's result was not redundant.

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

In that case, no problem.
In that context, your saying printf is redundant makes sense ;-)
Will do.

Besides, that's more consistent with what's done e.g., in

  src/lxc_conf.h:    char *logDir;
  src/qemu_conf.h:    char *logDir;
  src/uml_conf.h:    char *logDir;