From mboxrd@z Thu Jan  1 00:00:00 1970
From: George Dunlap <george.dunlap@citrix.com>
Subject: Re: [PATCH V11 2/5] libxl_utils: add internal function
 to read sysfs file contents
Date: Tue, 15 Dec 2015 11:16:37 +0000
Message-ID: <566FF695.9030605@citrix.com>
References: <1450158901-5798-1-git-send-email-cyliu@suse.com>
	<1450158901-5798-3-git-send-email-cyliu@suse.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <1450158901-5798-3-git-send-email-cyliu@suse.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Chunyan Liu <cyliu@suse.com>, 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 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