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

[Pádraig Brady]

On 25/10/10 01:26, Pádraig Brady wrote:
> I noticed large realloc()s still being done by fread_file() for the
> common case of a regular file.  As well as being inefficient,
> it could needlessly fail for large files.
> 
> $ truncate -s1M t.f
> $ ltrace ./a.out < t.f
> malloc(1048577)                                  = 0xb77c0008
> fread(0xb77c0008, 1, 1048576, 0x7b7420)          = 1048576
> realloc(0xb77c0008, 1572865)                     = 0xb763f008
> fread(0xb773f008, 1, 524288, 0x7b7420)           = 0
> realloc(0xb763f008, 1048577)                     = 0xb763f008

> On a related note I was wondering if we should fall back
> to increasing the buffer by +=BUFSIZ rather than *=2
> when we get ENOMEM? If we do then we'd need to be more careful
> with the overflow checking with lines like the following:
> 
> if (size + BUFSIZ + 1 > alloc)

The above overflow issue might trigger with a large
file that subsequently increases in size.
I noticed a couple of other issues with the code so I
simplified/rearranged it further as per the attached.

In summary the changes are:

* Don't do 2 unnecessary reallocs for regular files
* Keep allocation to multiples of BUFSIZ which may be more efficient
* Allow allocating up to SIZE_MAX for streams, rather than about SIZE_MAX - 
SIZE_MAX/5
* Fix possible overflow causing invalid operation
* Explicitly check for overflow rather than using a roll over variable

For my own reference, I notice that even though
we now do a single fread() for a regular file,
we still get 3 read syscalls. I.E.

  fread(0xb77e9008, 1, 512002, 0xd52440)  = 512001

results in:

  read(0, "\0"..., 512000) = 512000
  read(0, "\0", 4096)                     = 1
  read(0, "", 4096)                       = 0

cheers,
Pádraig.

read-file.diff
Description: Text Data