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

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,