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

[Jim Meyering <jim meyering net>]

Guido Günther <agx sigxcpu org> wrote:
> attached patch splits out the qemu logfile opening into a separate
> function which makes the code a bit more readable and I'll need this for
> the libvirtd restart code.
...
> +static int
> +qemudLogFD(virConnectPtr conn, const char* logDir, const char* name)
> +{
> +    char logfile[PATH_MAX];
> +    mode_t logmode;
> +    uid_t uid = geteuid();
> +    int fd = -1;
> +
> +    if ((strlen(logDir) + /* path */
> +         1 + /* Separator */
> +         strlen(name) + /* basename */
> +         4 + /* suffix .log */
> +         1 /* NULL */) > PATH_MAX) {
> +        qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
> +                         _("config file path too long: %s/%s.log"),
> +                         logDir, name);
> +        return -1;
> +    }
> +
> +    strcpy(logfile, logDir);
> +    strcat(logfile, "/");
> +    strcat(logfile, name);
> +    strcat(logfile, ".log");

Since this is just moving code, ok.
Otherwise, I'd ask that the strlen check and strcpy/strcat
calls be replaced by a single checked snprintf call, using
"sizeof logfile" as the length, rather than duplicating PATH_MAX.