Re: [dmidecode] [PATCH 1/4] add read_file() function for reading sysfs files

[Jean Delvare]

Hi Roy,

On Thu, 16 Apr 2015 21:13:58 -0700, Roy Franz wrote:
> Add a function that can read a complete, unknown size file for
> reading entry point files from sysfs.  This function models its signature
> on the mem_chunk() funtion, so it also allocates memory that the caller
> needs to free.  The files that we are interested in reading are very small,
> and have a known upper bound on the size.  The EINTR handling is based
> on the myread() function.
> 
> Signed-off-by: Roy Franz <address@hidden>
> ---
>  util.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  util.h |  1 +
>  2 files changed, 51 insertions(+)
> 
> diff --git a/util.c b/util.c
> index 2ab4915..8acc8f5 100644
> --- a/util.c
> +++ b/util.c
> @@ -77,6 +77,56 @@ static int myread(int fd, u8 *buf, size_t count, const 
> char *prefix)
>  
>       return 0;
>  }
> +/*
> + * Reads all of file, up to len bytes.
> + * A buffer of len bytes is allocated by this function, and
> + * needs to be freed by the caller.
> + * This provides a similar usage model to mem_chunk()
> + *
> + * Returns pointer to buffer of len bytes, or NULL on error
> + *
> + */
> +void *read_file(size_t len, const char *filename)

Maybe rename len to max_len for clarity?

> +{
> +     int fd;
> +     size_t r2 = 0;
> +     ssize_t r;
> +     u8 *p;
> +
> +     /*
> +      * Don't print error message on missing file, as we will try to read
> +      * files that may or may not be present.
> +      */

Keeping silent on errno == ENOENT is fine indeed, but maybe other
errors should be reported? For example if running as non-root, you'll
get EACCES (I think) and it would be better to let the user know and
maybe even stop there (as reading from /dev/mem won't work either.)

I'm fine with doing this in a separate patch though.

> +     if ((fd = open(filename, O_RDONLY)) == -1)
> +             return NULL;
> +
> +     if ((p = malloc(len)) == NULL)
> +     {
> +             perror("malloc");
> +             return NULL;
> +     }
> +
> +     do
> +     {
> +             r = read(fd, p + r2, len - r2);
> +             if (r == -1)
> +             {
> +                     if (errno != EINTR)
> +                     {
> +                             close(fd);
> +                             perror(filename);
> +                             free(p);
> +                             return NULL;
> +                     }
> +             }
> +             else
> +                     r2 += r;
> +     }
> +     while (r != 0);
> +
> +     close(fd);
> +     return p;
> +}
>  
>  int checksum(const u8 *buf, size_t len)
>  {
> diff --git a/util.h b/util.h
> index e564292..ca8080e 100644
> --- a/util.h
> +++ b/util.h
> @@ -28,3 +28,4 @@ int checksum(const u8 *buf, size_t len);
>  void *mem_chunk(size_t base, size_t len, const char *devmem);
>  int write_dump(size_t base, size_t len, const void *data, const char 
> *dumpfile, int add);
>  u64 u64_range(u64 start, u64 end);
> +void *read_file(size_t len, const char *filename);


-- 
Jean Delvare
SUSE L3 Support