Re: [Xen-devel] [PATCH V6 2/7] libxl_read_file_contents: add new entry to read sysfs file

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Mon, Aug 10, 2015 at 06:35:23PM +0800, 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@xxxxxxxx>
> 
> ---
> Changes:
>   - read one more byte to check bigger size problem.
> 
>  tools/libxl/libxl_internal.h |  2 ++
>  tools/libxl/libxl_utils.c    | 51 
> ++++++++++++++++++++++++++++++++++----------
>  2 files changed, 42 insertions(+), 11 deletions(-)
> 
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 6013628..f98f089 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -4001,6 +4001,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);

Indentation looks wrong.

>  
>  /*
>   * Local variables:
> diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
> index bfc9699..9234efb 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,34 @@ 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;
> +            }

This is a bit bikeshedding, but you can leave "goto xe" out of two `if'
to reduce patch size.

>          }
> +
> +        data = realloc(data, datalen);

Should check return value of realloc.

The logic of this function reflects what has been discussed so far.

Wei.

>      }
>  
>      if (fclose(f)) {
> @@ -396,6 +412,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,                 \
> -- 
> 2.1.4

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel