[libvirt] [PATCH] util.c: add a file-descriptor-based wrapper for fread_file_lim

"Daniel P. Berrange" <berrange(a)redhat.com&gt; wrote: Here are two change sets: The first adds two new file-reading functions in util.c and makes the existing virFileReadAll use one of them. The second implements the change suggested above. From: Jim Meyering <meyering(a)redhat.com&gt; Date: Thu, 28 Aug 2008 15:54:03 +0200 Subject: [PATCH] util.c: add a file-descriptor-based wrapper for fread_file_lim * src/util.c (virFileReadLimFP): New function. (__virFileReadLimFD): New function. * src/util.h (__virFileReadLimFD): Declare. (virFileReadLimFD): Define. (virFileReadAll): Rewrite to use virFileReadLimFP. --- src/util.c | 71 +++++++++++++++++++++++++++++++++++++++-------------------- src/util.h | 7 +++-- 2 files changed, 51 insertions(+), 27 deletions(-) diff --git a/src/util.c b/src/util.c index cb03e7b..c724268 100644 --- a/src/util.c +++ b/src/util.c @@ -510,40 +510,63 @@ fread_file_lim (FILE *stream, size_t max_len, size_t *length) return NULL; } -int __virFileReadAll(const char *path, int maxlen, char **buf) +/* A wrapper around fread_file_lim that maps a failure due to + exceeding the maximum size limitation to EOVERFLOW. */ +static int virFileReadLimFP(FILE *fp, int maxlen, char **buf) { - FILE *fh; - int ret = -1; size_t len; - char *s; + char *s = fread_file_lim (fp, maxlen+1, &len); + if (s == NULL) + return -1; + if (len > maxlen || (int)len != len) { + VIR_FREE(s); + /* There was at least one byte more than MAXLEN. + Set errno accordingly. */ + errno = EOVERFLOW; + return -1; + } + *buf = s; + return len; +} + +/* Like virFileReadLimFP, but use a file descriptor rather than a FILE*. */ +int __virFileReadLimFD(int fd_arg, int maxlen, char **buf) +{ + int fd = dup (fd_arg); + if (fd >= 0) { + FILE *fp = fdopen (fd, "r"); + if (fp) { + int len = virFileReadLimFP (fp, maxlen, buf); + int saved_errno = errno; + fclose (fp); + errno = saved_errno; + return len; + } else { + int saved_errno = errno; + close (fd); + errno = saved_errno; + } + } + return -1; +} - if (!(fh = fopen(path, "r"))) { +int __virFileReadAll(const char *path, int maxlen, char **buf) +{ + FILE *fh = fopen(path, "r"); + if (fh == NULL) { virLog("Failed to open file '%s': %s\n", path, strerror(errno)); - goto error; + return -1; } - s = fread_file_lim(fh, maxlen+1, &len); - if (s == NULL) { + int len = virFileReadLimFP (fh, maxlen, buf); + fclose(fh); + if (len < 0) { virLog("Failed to read '%s': %s\n", path, strerror (errno)); - goto error; - } - - if (len > maxlen || (int)len != len) { - VIR_FREE(s); - virLog("File '%s' is too large %d, max %d\n", - path, (int)len, maxlen); - goto error; + return -1; } - *buf = s; - ret = len; - - error: - if (fh) - fclose(fh); - - return ret; + return len; } int virFileMatchesNameSuffix(const char *file, diff --git a/src/util.h b/src/util.h index 5151582..f2da006 100644 --- a/src/util.h +++ b/src/util.h @@ -45,9 +45,10 @@ int virExec(virConnectPtr conn, int flags); int virRun(virConnectPtr conn, const char *const*argv, int *status); -int __virFileReadAll(const char *path, - int maxlen, - char **buf); +int __virFileReadLimFD(int fd, int maxlen, char **buf); +#define virFileReadLimFD(fd,m,b) __virFileReadLimFD((fd),(m),(b)) + +int __virFileReadAll(const char *path, int maxlen, char **buf); #define virFileReadAll(p,m,b) __virFileReadAll((p),(m),(b)) int virFileMatchesNameSuffix(const char *file, -- 1.6.0.1.126.ga118 From: Jim Meyering <meyering(a)redhat.com&gt; Date: Fri, 29 Aug 2008 14:43:34 +0200 Subject: [PATCH] virFileReadLimFD, virFileReadLimFP: new functions; use them * src/qemu_conf.c (qemudExtractVersionInfo): Use virFileReadLimFD and VIR_FREE in place of an open-coded loop and a static buffer. --- src/qemu_conf.c | 21 +++++++-------------- 1 files changed, 7 insertions(+), 14 deletions(-) diff --git a/src/qemu_conf.c b/src/qemu_conf.c index bab2092..0686978 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -401,8 +401,7 @@ int qemudExtractVersionInfo(const char *qemu, const char *const qemuenv[] = { "LC_ALL=C", NULL }; pid_t child; int newstdout = -1; - char help[8192]; /* Ought to be enough to hold QEMU help screen */ - int got = 0, ret = -1, status; + int ret = -1, status; unsigned int major, minor, micro; unsigned int version; unsigned int flags = 0; @@ -416,16 +415,11 @@ int qemudExtractVersionInfo(const char *qemu, &child, -1, &newstdout, NULL, VIR_EXEC_NONE) < 0) return -1; - - while (got < (sizeof(help)-1)) { - int len; - if ((len = saferead(newstdout, help+got, sizeof(help)-got-1)) < 0) - goto cleanup2; - if (!len) - break; - got += len; - } - help[got] = '\0'; + char *help = NULL; + enum { MAX_HELP_OUTPUT_SIZE = 8192 }; + int len = virFileReadLimFD(newstdout, MAX_HELP_OUTPUT_SIZE, &help); + if (len < 0) + goto cleanup2; if (sscanf(help, "QEMU PC emulator version %u.%u.%u", &major, &minor, &micro) != 3) { @@ -447,7 +441,6 @@ int qemudExtractVersionInfo(const char *qemu, if (version >= 9000) flags |= QEMUD_CMD_FLAG_VNC_COLON; - if (retversion) *retversion = version; if (retflags) @@ -459,6 +452,7 @@ int qemudExtractVersionInfo(const char *qemu, major, minor, micro, version, flags); cleanup2: + VIR_FREE(help); if (close(newstdout) < 0) ret = -1; @@ -1229,4 +1223,3 @@ int qemudBuildCommandLine(virConnectPtr conn, #undef ADD_ARG_LIT #undef ADD_ARG_SPACE } - -- 1.6.0.1.126.ga118