From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [PATCH V7 2/7] libxl_read_file_contents: add new entry to read sysfs file Date: Wed, 30 Sep 2015 12:22:37 +0100 Message-ID: <560BC5FD.2080102@citrix.com> References: <1443147102-6471-1-git-send-email-cyliu@suse.com> <1443147102-6471-3-git-send-email-cyliu@suse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1443147102-6471-3-git-send-email-cyliu@suse.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Chunyan Liu , xen-devel@lists.xen.org Cc: jgross@suse.com, wei.liu2@citrix.com, ian.campbell@citrix.com, george.dunlap@eu.citrix.com, Ian.Jackson@eu.citrix.com, jfehlig@suse.com List-Id: xen-devel@lists.xenproject.org On 25/09/15 03:11, Chunyan Liu wrote: > Sysfs file has size=4096 but actual file content is less than that. > Current libxl_read_file_contents will treat it as error when file size > and actual file content differs, so reading sysfs file content with > this function always fails. > > Add a new entry libxl_read_sysfs_file_contents to handle sysfs file > specially. It would be used in later pvusb work. > > Signed-off-by: Chunyan Liu > Reviewed-by: Wei Liu This seems like it's a generic enough improvement to stand on its own -- would it make sense to check this in regardless, so we can remove it from the series? -George > --- > Changes: > - check return value after realloc > > tools/libxl/libxl_internal.h | 2 ++ > tools/libxl/libxl_utils.c | 52 ++++++++++++++++++++++++++++++++++---------- > 2 files changed, 43 insertions(+), 11 deletions(-) > > diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h > index 6ff17f9..1a2e483 100644 > --- a/tools/libxl/libxl_internal.h > +++ b/tools/libxl/libxl_internal.h > @@ -4015,6 +4015,8 @@ void libxl__bitmap_copy_best_effort(libxl__gc *gc, libxl_bitmap *dptr, > > int libxl__count_physical_sockets(libxl__gc *gc, int *sockets); > #endif > +_hidden int libxl_read_sysfs_file_contents(libxl_ctx *ctx, const char *filename, > + void **data_r, int *datalen_r); > > /* > * Local variables: > diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c > index bfc9699..72f8f4f 100644 > --- a/tools/libxl/libxl_utils.c > +++ b/tools/libxl/libxl_utils.c > @@ -322,8 +322,10 @@ out: > return rc; > } > > -int libxl_read_file_contents(libxl_ctx *ctx, const char *filename, > - void **data_r, int *datalen_r) { > +static int libxl_read_file_contents_core(libxl_ctx *ctx, const char *filename, > + void **data_r, int *datalen_r, > + bool tolerate_shrinking_file) > +{ > GC_INIT(ctx); > FILE *f = 0; > uint8_t *data = 0; > @@ -359,20 +361,35 @@ int libxl_read_file_contents(libxl_ctx *ctx, const char *filename, > datalen = stab.st_size; > > if (stab.st_size && data_r) { > - data = malloc(datalen); > + data = malloc(datalen + 1); > if (!data) goto xe; > > - rs = fread(data, 1, datalen, f); > - if (rs != datalen) { > - if (ferror(f)) > + rs = fread(data, 1, datalen + 1, f); > + if (rs > datalen) { > + LOG(ERROR, "%s increased size while we were reading it", > + filename); > + goto xe; > + } > + > + if (rs < datalen) { > + if (ferror(f)) { > LOGE(ERROR, "failed to read %s", filename); > - else if (feof(f)) > - LOG(ERROR, "%s changed size while we were reading it", > - filename); > - else > + goto xe; > + } else if (feof(f)) { > + if (tolerate_shrinking_file) { > + datalen = rs; > + } else { > + LOG(ERROR, "%s shrunk size while we were reading it", > + filename); > + goto xe; > + } > + } else { > abort(); > - goto xe; > + } > } > + > + data = realloc(data, datalen); > + if (!data) goto xe; > } > > if (fclose(f)) { > @@ -396,6 +413,19 @@ int libxl_read_file_contents(libxl_ctx *ctx, const char *filename, > return e; > } > > +int libxl_read_file_contents(libxl_ctx *ctx, const char *filename, > + void **data_r, int *datalen_r) > +{ > + return libxl_read_file_contents_core(ctx, filename, data_r, datalen_r, 0); > +} > + > +int libxl_read_sysfs_file_contents(libxl_ctx *ctx, const char *filename, > + void **data_r, int *datalen_r) > +{ > + return libxl_read_file_contents_core(ctx, filename, data_r, datalen_r, 1); > +} > + > + > #define READ_WRITE_EXACTLY(rw, zero_is_eof, constdata) \ > \ > int libxl_##rw##_exactly(libxl_ctx *ctx, int fd, \ >