[libvirt] [PATCH] split out logfile opening

[Jim Meyering]

Guido Günther <agx at sigxcpu.org> wrote:
>> Does the attached patch look ok? I'll apply this together with
>> 0001-split-out-opening-of-the-domain-logfile.patch then.
>>  -- Guido
> Rereading your mail you also suggest using sizeof() instead of PATH_MAX.
> Updated patch attached.
>  -- Guido
>
>>From 6ca6494be05e4834b9469ec1c8a108cefe3ed44f Mon Sep 17 00:00:00 2001
> From: =?utf-8?q?Guido=20G=C3=BCnther?= <agx at sigxcpu.org>
> Date: Sat, 10 Jan 2009 20:13:39 +0100
> Subject: [PATCH] use snprintf
>
> ---
>  src/qemu_driver.c |   19 ++++++++-----------
>  1 files changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/src/qemu_driver.c b/src/qemu_driver.c
> index d4c56d6..fc73a12 100644
> --- a/src/qemu_driver.c
> +++ b/src/qemu_driver.c
> @@ -149,24 +149,21 @@ qemudLogFD(virConnectPtr conn, const char* logDir, const char* name)
>      char logfile[PATH_MAX];
>      mode_t logmode;
>      uid_t uid = geteuid();
> -    int fd = -1;
> +    int ret, fd = -1;
>
> -    if ((strlen(logDir) + /* path */
> -         1 + /* Separator */
> -         strlen(name) + /* basename */
> -         4 + /* suffix .log */
> -         1 /* NULL */) > PATH_MAX) {
> +    if ((ret = snprintf(logfile, sizeof(logfile), "%s/%s.log", logDir, name)) < 0) {
> +        qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
> +                         _("failed to build logfile name %s/%s.log"),
> +                         logDir, name);
> +        return -1;
> +    }
> +    if (ret >= sizeof(logfile)) {
>          qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
>                           _("config file path too long: %s/%s.log"),
>                           logDir, name);
>          return -1;
>      }

Thanks!
That looks fine.
However, it makes me see the 2nd diagnostic is wrong
to mention "config file".  Should be "log file".
Yet again, this was a preexisting error, not yours, of course.

Maybe just drop the "path too long" case (which will probably
never happen anyway) and combine the tests, also splitting the
line to fit in 80 columns?

    if ((ret = snprintf(logfile, sizeof(logfile), "%s/%s.log", logDir, name))
        < 0 || ret >= sizeof(logfile)) {
        qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
                         _("failed to build logfile name %s/%s.log"),
                         logDir, name);
        return -1;
    }