io/ioutil: don't cap buffer size in ReadFile

When we added a Stat call to determine the initial buffer size in
https://golang.org/cl/163069, we included an arbitrary 1e9-byte limit
"just in case". That interacts badly with power-of-2 resizing in
*bytes.Buffer: it causes buffers reading from very large files to
consume up to twice the necessary space.

The documentation for (os.FileInfo).Size says that it reports "length
in bytes for regular files; system-dependent for others", but the
"system dependent" cases overwhelmingly return either a small number
(e.g., the length of the target path for a symlink) or a non-positive
number (e.g., for a file in /proc under Linux). It should be
appropriate to use the number reported by Size as an approximate lower
bound, even if it is large.

fixes #21455

Change-Id: I609c72519b7b87428c24d0b22db46eede30e0e54
Reviewed-on: https://go-review.googlesource.com/55870Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

src/io/ioutil/ioutil.go

@@ -16,7 +16,7 @@ import (
 // readAll reads from r until an error or EOF and returns the data it read
 // from the internal buffer allocated with a specified capacity.
 func readAll(r io.Reader, capacity int64) (b []byte, err error) {
-	buf := bytes.NewBuffer(make([]byte, 0, capacity))
+	var buf bytes.Buffer
 	// If the buffer overflows, we will get bytes.ErrTooLarge.
 	// Return that as an error. Any other panic remains.
 	defer func() {
@@ -30,6 +30,9 @@ func readAll(r io.Reader, capacity int64) (b []byte, err error) {
 			panic(e)
 		}
 	}()
+	if int64(int(capacity)) == capacity {
+		buf.Grow(int(capacity))
+	}
 	_, err = buf.ReadFrom(r)
 	return buf.Bytes(), err
 }
@@ -54,20 +57,20 @@ func ReadFile(filename string) ([]byte, error) {
 	defer f.Close()
 	// It's a good but not certain bet that FileInfo will tell us exactly how much to
 	// read, so let's try it but be prepared for the answer to be wrong.
-	var n int64
+	var n int64 = bytes.MinRead
 
 	if fi, err := f.Stat(); err == nil {
-		// Don't preallocate a huge buffer, just in case.
-		if size := fi.Size(); size < 1e9 {
+		// As initial capacity for readAll, use Size + a little extra in case Size
+		// is zero, and to avoid another allocation after Read has filled the
+		// buffer. The readAll call will read into its allocated internal buffer
+		// cheaply. If the size was wrong, we'll either waste some space off the end
+		// or reallocate as needed, but in the overwhelmingly common case we'll get
+		// it just right.
+		if size := fi.Size() + bytes.MinRead; size > n {
 			n = size
 		}
 	}
-	// As initial capacity for readAll, use n + a little extra in case Size is zero,
-	// and to avoid another allocation after Read has filled the buffer. The readAll
-	// call will read into its allocated internal buffer cheaply. If the size was
-	// wrong, we'll either waste some space off the end or reallocate as needed, but
-	// in the overwhelmingly common case we'll get it just right.
-	return readAll(f, n+bytes.MinRead)
+	return readAll(f, n)
 }
 
 // WriteFile writes data to a file named by filename.