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 :|
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list