On Mon, Jan 05, 2009 at 07:52:26AM +0100, Jim Meyering wrote:
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.
I agree with Jim, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/