Re: [PATCH v5 2/4] util: Introduce read_file() and read_file_integer() function

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 4844 bytes --]

On Fri, Oct 17, 2025 at 12:27:41PM +0800, Yumei Huang wrote:
> Signed-off-by: Yumei Huang <yuhuang@redhat.com>
> ---
>  util.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  util.h | 10 +++++++
>  2 files changed, 94 insertions(+)
> 
> diff --git a/util.c b/util.c
> index c492f90..ff21bf6 100644
> --- a/util.c
> +++ b/util.c
> @@ -579,6 +579,90 @@ int write_file(const char *path, const char *buf)
>  	return len == 0 ? 0 : -1;
>  }
>  
> +/**
> + * read_file() - Read contents of file into a buffer
> + * @path:	File to read
> + * @buf:	Buffer to store file contents
> + * @buf_size:	Size of buffer
> + *
> + * Return: number of bytes read on success, -1 on any error, -2 on truncation
> +*/
> +ssize_t read_file(const char *path, char *buf, size_t buf_size)
> +{
> +	int fd = open(path, O_RDONLY | O_CLOEXEC);
> +	size_t total_read = 0;
> +	ssize_t rc;
> +
> +	if (fd < 0) {
> +		warn_perror("Could not open %s", path);
> +		return -1;
> +	}
> +
> +	while (total_read < buf_size) {
> +		rc = read(fd, buf + total_read, buf_size - total_read);
> +
> +		if (rc < 0) {
> +			warn_perror("Couldn't read from %s", path);
> +			close(fd);
> +			return -1;
> +		}
> +
> +		if (rc == 0)
> +			break;
> +
> +		total_read += rc;
> +	}
> +
> +	close(fd);
> +
> +	if (total_read == buf_size) {
> +		warn_perror("File %s truncated, buffer too small", path);

Sorry, I should have caught this in the last version.  You don't want
warn_perror() here, just warn().  warn_perror() includes an error
reason based on errno, but errno is not meaningful here (it will
almost certainly be 0 from the close()).

> +		return -2;
> +	}
> +
> +	buf[total_read] = '\0';
> +
> +	return total_read;
> +}
> +
> +/**
> + * read_file_integer() - Read an integer value from a file
> + * @path: File to read
> + * @fallback: Default value if file can't be read
> + *
> + * Return: Integer value, fallback on failure
> +*/
> +intmax_t read_file_integer(const char *path, intmax_t fallback)
> +{
> +	char buf[INTMAX_STRLEN];
> +	ssize_t bytes_read;
> +	intmax_t value;
> +	char *end;
> +
> +	bytes_read = read_file(path, buf, sizeof(buf));
> +
> +	if (bytes_read < 0)
> +		return fallback;
> +
> +	if (bytes_read == 0) {
> +		debug("Empty file %s", path);
> +		return fallback;
> +	}
> +
> +	errno = 0;
> +	value = strtoimax(buf, &end, 10);
> +	if (*end && *end != '\n') {
> +		debug("Invalid format in %s", path);
> +		return fallback;
> +	}
> +	if (errno) {
> +		debug("Invalid value in %s: %s", path, buf);
> +		return fallback;
> +	}
> +
> +	return value;
> +}
> +
>  #ifdef __ia64__
>  /* Needed by do_clone() below: glibc doesn't export the prototype of __clone2(),
>   * use the description from clone(2).
> diff --git a/util.h b/util.h
> index 22eaac5..acfbd08 100644
> --- a/util.h
> +++ b/util.h
> @@ -222,6 +222,8 @@ void pidfile_write(int fd, pid_t pid);
>  int __daemon(int pidfile_fd, int devnull_fd);
>  int fls(unsigned long x);
>  int write_file(const char *path, const char *buf);
> +ssize_t read_file(const char *path, char *buf, size_t buf_size);
> +intmax_t read_file_integer(const char *path, intmax_t fallback);
>  int write_all_buf(int fd, const void *buf, size_t len);
>  int write_remainder(int fd, const struct iovec *iov, size_t iovcnt, size_t skip);
>  int read_all_buf(int fd, void *buf, size_t len);
> @@ -250,6 +252,14 @@ static inline const char *af_name(sa_family_t af)
>  
>  #define UINT16_STRLEN		(sizeof("65535"))
>  
> +/* Each byte holds roughly 2.4 decimal digits:
> + *    1 bit = log10(2) ~= 0.30103 decimal digits
> + *    1 byte = 8 bits = 8 x 0.30103 ~= 2.408 decimal digits

I don't think these calculations are particularly useful to a reader.
The key point is that each byte expands to at most 3 decimal digits -
since 0xff == 255, I don't think we need to talk about logs to
establish that.

In any case, the main reason for a comment here is not to explain the
logic, but to give credit to where it came from.  You got it from
Claude, but Claude's info was so similar to the stack overflow page
Stefano mentioned, it almost certainly took it from there.  So, I'd
include a link to that page.

> + * Using sizeof(intmax_t) * 3 provides a safe upper bound,
> + * plus 2 extra bytes for the sign and null terminator.
> + */
> +#define INTMAX_STRLEN		(sizeof(intmax_t) * 3 + 2)
> +
>  /* inet address (- '\0') + port (u16) (- '\0') + ':' + '\0' */
>  #define SOCKADDR_INET_STRLEN					\
>  	(INET_ADDRSTRLEN-1 + UINT16_STRLEN-1 + sizeof(":"))
> -- 
> 2.47.0
> 

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]