Re: [PATCH v2 1/7] Introduce read_entire_file

[Simon Marchi <simark@simark.ca>]

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