Re: [libvirt] [PATCH] split out logfile opening

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Guido Günther <agx@xxxxxxxxxxx> 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@xxxxxxxxxxx>
> 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;
    }

--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]