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

On Tue, Jan 06, 2009 at 03:28:59PM +0100, Daniel Veillard wrote: > On Mon, Jan 05, 2009 at 07:52:26AM +0100, Jim Meyering wrote: > > Guido Günther <agx(a)sigxcpu.org&gt; 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. > > I agree with Jim, I only copied the code into a separate function but I can fix this up too while at it.