Re: [PATCH V11 2/5] libxl_utils: add internal function to read sysfs file contents

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

On 15/12/15 05:54, Chunyan Liu wrote:
> Add a new function 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>
> ---
>  tools/libxl/libxl_internal.h |  4 +++
>  tools/libxl/libxl_utils.c    | 77 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 81 insertions(+)
> 
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index beaef3f..6b873c7 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -4026,6 +4026,10 @@ void libxl__bitmap_copy_best_effort(libxl__gc *gc, libxl_bitmap *dptr,
>  
>  int libxl__count_physical_sockets(libxl__gc *gc, int *sockets);
>  
> +_hidden int libxl__read_sysfs_file_contents(libxl__gc *gc,
> +                                            const char *filename,
> +                                            void **data_r,
> +                                            int *datalen_r);
>  
>  #define LIBXL_QEMU_USER_PREFIX "xen-qemuuser"
>  #define LIBXL_QEMU_USER_BASE   LIBXL_QEMU_USER_PREFIX"-domid"
> diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
> index e42422a..7f612a6 100644
> --- a/tools/libxl/libxl_utils.c
> +++ b/tools/libxl/libxl_utils.c
> @@ -396,6 +396,83 @@ int libxl_read_file_contents(libxl_ctx *ctx, const char *filename,
>      return e;
>  }
>  
> +int libxl__read_sysfs_file_contents(libxl__gc *gc, const char *filename,
> +                                    void **data_r, int *datalen_r)
> +{
> +    FILE *f = 0;
> +    uint8_t *data = 0;
> +    int datalen = 0;
> +    int e;
> +    struct stat stab;
> +    ssize_t rs;
> +
> +    f = fopen(filename, "r");
> +    if (!f) {
> +        if (errno == ENOENT) return ENOENT;
> +        LOGE(ERROR, "failed to open %s", filename);
> +        goto xe;
> +    }
> +
> +    if (fstat(fileno(f), &stab)) {
> +        LOGE(ERROR, "failed to fstat %s", filename);
> +        goto xe;
> +    }
> +
> +    if (!S_ISREG(stab.st_mode)) {
> +        LOGE(ERROR, "%s is not a plain file", filename);
> +        errno = ENOTTY;
> +        goto xe;
> +    }
> +
> +    if (stab.st_size > INT_MAX) {
> +        LOG(ERROR, "file %s is far too large", filename);
> +        errno = EFBIG;
> +        goto xe;
> +    }
> +
> +    datalen = stab.st_size;
> +
> +    if (stab.st_size && data_r) {
> +        data = libxl__malloc(gc, datalen);
> +        if (!data) goto xe;

I think the libxl allocation functions never return NULL (if the malloc
fails, the function itself will crash the whole process).  So yu don't
need to do these null-checks here after libxl__malloc() and
libxl__realloc().

Everything else looks OK to me.

 -George