Re: [libvirt] PATCH: 5/5: Make all code use virExec

"Daniel P. Berrange" <berrange(a)redhat.com&gt; wrote: > On Wed, Aug 20, 2008 at 06:40:37PM +0200, Jim Meyering wrote: >> "Daniel P. Berrange" <berrange(a)redhat.com&gt; wrote: > + > + > + 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'; I was going to ask why you are using saferead, then realized that *I* suggested the s/read+EINTR/saferead/ change. Now, while re-reviewing this, I wondered if we could get rid of the 8KB stack buffer and encapsulate the above loop -- at the expense of allocating the memory instead -- by using e.g., virFileReadAll. But virFileReadAll operates on a file name, not a file descriptor. So I wrote/factored a couple of wrappers, and now, with the following patch, you can use this: char *help = NULL; enum { MAX_HELP_OUTPUT_SIZE = 8192 }; int len = virFileReadLimFD(newstdout, MAX_HELP_OUTPUT_SIZE, &help); if (len < 0) goto ... ... Then add this somewhere after done with "help": VIR_FREE(help);

diff --git a/src/util.c b/src/util.c index a81af07..fd30778 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 (0 <= fd) {