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@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, I only copied the code into a separate function but I can fix this up too while at it. -- Guido