On Mon, 2009-10-12 at 20:37 +0100, Daniel P. Berrange wrote:
The fread_file_lim() function uses fread() but never handles
EINTR results, causing unexpected failures when reading QEMU
help arg info. It was unneccessarily using FILE * instead
of plain UNIX file handles, which prevented use of saferead()
Looks like the same thing Charles Duffy reported here:
http://www.redhat.com/archives/libvir-list/2009-September/msg00662.html
* src/util/util.c: Switch fread_file_lim over to use saferead
instead of fread, remove FILE * use, and rename
---
src/util/util.c | 45 ++++++++++++---------------------------------
1 files changed, 12 insertions(+), 33 deletions(-)
diff --git a/src/util/util.c b/src/util/util.c
index e5135fc..98f8a14 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -898,7 +898,7 @@ virExec(virConnectPtr conn,
number of bytes. If the length of the input is <= max_len, and
upon error while reading that data, it works just like fread_file. */
static char *
-fread_file_lim (FILE *stream, size_t max_len, size_t *length)
+saferead_lim (int fd, size_t max_len, size_t *length)
{
char *buf = NULL;
size_t alloc = 0;
@@ -906,8 +906,8 @@ fread_file_lim (FILE *stream, size_t max_len, size_t *length)
int save_errno;
for (;;) {
- size_t count;
- size_t requested;
+ int count;
+ int requested;
if (size + BUFSIZ + 1 > alloc) {
alloc += alloc / 2;
@@ -923,12 +923,12 @@ fread_file_lim (FILE *stream, size_t max_len, size_t *length)
/* Ensure that (size + requested <= max_len); */
requested = MIN (size < max_len ? max_len - size : 0,
alloc - size - 1);
- count = fread (buf + size, 1, requested, stream);
+ count = saferead (fd, buf + size, requested);
size += count;
if (count != requested || requested == 0) {
save_errno = errno;
- if (ferror (stream))
+ if (count < 0)
break;
Perhaps you should move the save_errno assignment under this condition
too, but not a big deal
buf[size] = '\0';
*length = size;
@@ -941,12 +941,12 @@ fread_file_lim (FILE *stream, size_t max_len, size_t *length)
return NULL;
}
-/* A wrapper around fread_file_lim that maps a failure due to
+/* A wrapper around saferead_lim that maps a failure due to
exceeding the maximum size limitation to EOVERFLOW. */
-static int virFileReadLimFP(FILE *fp, int maxlen, char **buf)
+int virFileReadLimFD(int fd, int maxlen, char **buf)
{
size_t len;
- char *s = fread_file_lim (fp, maxlen+1, &len);
+ char *s = saferead_lim (fd, maxlen+1, &len);
if (s == NULL)
return -1;
if (len > maxlen || (int)len != len) {
@@ -960,37 +960,16 @@ static int virFileReadLimFP(FILE *fp, int maxlen, char **buf)
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;
-}
-
int virFileReadAll(const char *path, int maxlen, char **buf)
{
- FILE *fh = fopen(path, "r");
- if (fh == NULL) {
+ int fd = open(path, O_RDONLY);
+ if (fd < 0) {
virReportSystemError(NULL, errno, _("Failed to open file
'%s'"), path);
return -1;
}
- int len = virFileReadLimFP (fh, maxlen, buf);
- fclose(fh);
+ int len = virFileReadLimFD(fd, maxlen, buf);
+ close(fd);
if (len < 0) {
virReportSystemError(NULL, errno, _("Failed to read file
'%s'"), path);
return -1;
Looks good to me, ACK
Cheers,
Mark.