Bruno Haible <[EMAIL PROTECTED]> writes:
>> Index: modules/strfile
>
> I would call the module 'read-file' and the function 'read_file'. So that it's
> possible to add a module 'write_file' if needed, and for consistency with the
> 'copy-file' module.
I like this, and I also like your fread_file function. I will create
a module read-file with two functions fread_file and read_file.
>> + do {
>
> In GNU style the brace goes on the next line.
I'll indent it.
Hmmm. I've made the same mistake before. I'll see how hard it would
be to write a tool that will "prepare" additions of a gnulib module,
for one it should run indent on the files, and then run 'cvs
--new-file diff' on the modules file, and the files mentioned in the
modules file. Eventually, it could apply the tests from the
maintainer-makefile module.
>> + tmp = realloc (out, pos + BUFSIZ);
>
> Quadratic runtime behaviour: if you don't have particular luck with the
> realloc()
> implementation, for large files, this loop will spend most of its time in
> realloc(), copying memory around.
Your fread_file may solve this, and read_file will use it.
>> + if (!(feof (fh) && fclose (fh)))
>
> Missing treatment of the ferror (fh) case.
Same here.
> As additional input, find attached some similar (unpolished) code I wrote
> long ago
> in the past. The good point about a function that takes a FILE stream rather
> than a filename as argument is that the caller has more liberty to decide
> about O_TEXT / O_BINARY and about what to do when the file does not exist.
Agreed.
> The downside is, of course, that it's not so immediate to use this
> function. But maybe the module can provide both: fread_file that
> reads from a stream, and read_file that takes a filename?
Exactly!
/Simon
