Re: strfile: new module

[Bruno Haible]

Simon Josefsson wrote:
> Here's the module.  Only lighted tested, mostly to get feedback on the
> approach, but if you agree with it, detailed review would be useful.

Looks already quite good. Only minor nits:

> +/* Read a STREAM and return a newly allocated string with the content,
> +   and set LENGTH to the length of the string.

It sets *LENGTH, not LENGTH, I think.

>                                                  The string is
> +   zero-terminated, but the terminating zero character is not counted

I'd prefer to write "zero byte" instead of "zero character", because a
character nowadays can generally be multibyte.

> +   in the LENGTH variable.  On errors, return NULL, sets errno and

On systems like mingw (which don't attempt POSIX compliance) errno is
not set when fread() fails.

> +   LENGTH is undefined. */

*LENGTH here as well.

> +       char *tmp;

I would rename this variable to 'new_buf'.

> +  buf[size] = '\0';

This crashes if the stream was initially already at EOF. In this case
you must explicitly malloc at least 1 byte.

> +static char *
> +internal_read_file (const char *filename, size_t * length, const char *mode)
> +{
> +  FILE *stream = fopen (filename, mode);
> +  char *out = NULL;

The NULL initializer is unnecessary.

> +/* Open and read the contents of FILENAME, and return a newly
> +   allocated string with the content, and set LENGTH to the length of
> +   the string.  The string is zero-terminated, but the terminating
> +   zero character is not counted in the LENGTH variable.  On errors,
> +   return NULL and sets errno.  */

Same comments as above regarding *LENGTH and errno.

> +char *
> +read_file (const char *filename, size_t * length)

> +char *
> +read_binary_file (const char *filename, size_t * length)

These functions are so similar, that I would merge them into one:

  char *
  read_file (const char *filename, size_t *length, bool textmode)

> +Depends-on:
> +realloc

You don't need this dependency, since the 2nd argument you pass to realloc()
is always positive.

Bruno