Re: [PATCH V7 2/7] libxl_read_file_contents: add new entry to read sysfs file

[George Dunlap <george.dunlap@citrix.com>]

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 <cyliu@suse.com>
> Reviewed-by: Wei Liu <wei.liu2@citrix.com>

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,                 \
>