Re: [PATCH] read-file: Avoid memory reallocations with seekable files.

[Bruno Haible]

Hi Giuseppe,

> +      long pos = ftell (stream);
> +      if (pos < 0)
> +        return NULL;

Not every regular file is seekable (think of files under /proc
on Linux). Therefore, of 'ftello' fails, it's better to not pre-allocate
a buffer but start reading from the stream, like for non-regular files.

> +      buf = malloc (alloc);
> +      if (!buf)
> +        return NULL;

Here, when malloc() fails, you want errno to be set to ENOMEM. For this
to be guaranteed, the module description needs to have a dependency on
'malloc-posix'.

Paul Eggert wrote:
> > +      alloc = st.st_size - pos + 1;
> 
> The assignment could overflow, since off_t might be
> wider than size_t.  This must be checked for

Yes. Additionally, pos may be > st.st_size on some files (think of
files like /proc/pid/maps on Linux). Check against that as well.

> and errno should be set to ENOMEM if overflow occurs.

I disagree. In this case there's likely something fishy with the fstat
results, and it's better to start reading from the stream, like for
non-regular files.

If you want to check against overflow, it should certainly also happen in
the loop, here:

          alloc += alloc / 2;
          if (alloc < size + BUFSIZ + 1)
            alloc = size + BUFSIZ + 1;

In this case, returning with ENOMEM is appropriate.

Bruno