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

Guido Günther <agx(a)sigxcpu.org&gt; 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(a)sigxcpu.org&gt; > 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; }