Re: [PATCH v2 1/7] Introduce read_entire_file

Simon Marchi
2020-07-06T13:47:55Z

On 2020-06-23 9:20 a.m., Tom Tromey wrote:
> +std::string
> +read_entire_file (const char *name, int fd, time_t *mtime)
> +{
> + struct stat st;
> + if (fstat (fd, &st) < 0)
> + perror_with_name (name);
> + size_t len = st.st_size;
> +
> + std::string lines;
> + lines.resize (len);
> + char *addr = &lines[0];
> +
> + while (len > 0)
> + {
> + int val = read (fd, addr, len);

Should we handle EINTR here?

> + if (val < 0)
> + perror_with_name (name);
> + if (val == 0)
> + break;
> + len -= val;
> + addr += val;
> + }

Would you mind adding newlines around the `if` statements? Maybe it's
just me, but I find it really more difficult to follow the code when it's
all packed. I would mean something like:

int val = read (fd, addr, len);

if (val < 0)
perror_with_name (name);

if (val == 0)
break;

len -= val;
addr += val;

Like this, my mind is more easily capable of seeing the logical steps.

Simon