Merge pull request #6232 from LemonBoy/fix-readall

std: Don't trust stat() size in readAllAlloc fns

lib/std/array_list.zig

@@ -46,7 +46,11 @@ pub fn ArrayListAligned(comptime T: type, comptime alignment: ?u29) type {
         /// Deinitialize with `deinit` or use `toOwnedSlice`.
         pub fn initCapacity(allocator: *Allocator, num: usize) !Self {
             var self = Self.init(allocator);
-            try self.ensureCapacity(num);
+
+            const new_memory = try self.allocator.allocAdvanced(T, alignment, num, .at_least);
+            self.items.ptr = new_memory.ptr;
+            self.capacity = new_memory.len;
+
             return self;
         }
 
@@ -366,7 +370,11 @@ pub fn ArrayListAlignedUnmanaged(comptime T: type, comptime alignment: ?u29) typ
         /// Deinitialize with `deinit` or use `toOwnedSlice`.
         pub fn initCapacity(allocator: *Allocator, num: usize) !Self {
             var self = Self{};
-            try self.ensureCapacity(allocator, num);
+
+            const new_memory = try self.allocator.allocAdvanced(T, alignment, num, .at_least);
+            self.items.ptr = new_memory.ptr;
+            self.capacity = new_memory.len;
+
             return self;
         }
 

lib/std/fs.zig

@@ -1437,26 +1437,32 @@ pub const Dir = struct {
     /// On success, caller owns returned buffer.
     /// If the file is larger than `max_bytes`, returns `error.FileTooBig`.
     pub fn readFileAlloc(self: Dir, allocator: *mem.Allocator, file_path: []const u8, max_bytes: usize) ![]u8 {
-        return self.readFileAllocOptions(allocator, file_path, max_bytes, @alignOf(u8), null);
+        return self.readFileAllocOptions(allocator, file_path, max_bytes, null, @alignOf(u8), null);
     }
 
     /// On success, caller owns returned buffer.
     /// If the file is larger than `max_bytes`, returns `error.FileTooBig`.
+    /// If `size_hint` is specified the initial buffer size is calculated using
+    /// that value, otherwise the effective file size is used instead.
     /// Allows specifying alignment and a sentinel value.
     pub fn readFileAllocOptions(
         self: Dir,
         allocator: *mem.Allocator,
         file_path: []const u8,
         max_bytes: usize,
+        size_hint: ?usize,
         comptime alignment: u29,
         comptime optional_sentinel: ?u8,
     ) !(if (optional_sentinel) |s| [:s]align(alignment) u8 else []align(alignment) u8) {
         var file = try self.openFile(file_path, .{});
         defer file.close();
 
-        const stat_size = try file.getEndPos();
+        // If the file size doesn't fit a usize it'll be certainly greater than
+        // `max_bytes`
+        const stat_size = size_hint orelse math.cast(usize, try file.getEndPos()) catch
+            return error.FileTooBig;
 
-        return file.readAllAllocOptions(allocator, stat_size, max_bytes, alignment, optional_sentinel);
+        return file.readToEndAllocOptions(allocator, max_bytes, stat_size, alignment, optional_sentinel);
     }
 
     pub const DeleteTreeError = error{

lib/std/fs/file.zig

@@ -363,31 +363,49 @@ pub const File = struct {
         try os.futimens(self.handle, &times);
     }
 
+    /// Reads all the bytes from the current position to the end of the file.
     /// On success, caller owns returned buffer.
     /// If the file is larger than `max_bytes`, returns `error.FileTooBig`.
-    pub fn readAllAlloc(self: File, allocator: *mem.Allocator, stat_size: u64, max_bytes: usize) ![]u8 {
-        return self.readAllAllocOptions(allocator, stat_size, max_bytes, @alignOf(u8), null);
+    pub fn readToEndAlloc(self: File, allocator: *mem.Allocator, max_bytes: usize) ![]u8 {
+        return self.readToEndAllocOptions(allocator, max_bytes, null, @alignOf(u8), null);
     }
 
+    /// Reads all the bytes from the current position to the end of the file.
     /// On success, caller owns returned buffer.
     /// If the file is larger than `max_bytes`, returns `error.FileTooBig`.
+    /// If `size_hint` is specified the initial buffer size is calculated using
+    /// that value, otherwise an arbitrary value is used instead.
     /// Allows specifying alignment and a sentinel value.
-    pub fn readAllAllocOptions(
+    pub fn readToEndAllocOptions(
         self: File,
         allocator: *mem.Allocator,
-        stat_size: u64,
         max_bytes: usize,
+        size_hint: ?usize,
         comptime alignment: u29,
         comptime optional_sentinel: ?u8,
     ) !(if (optional_sentinel) |s| [:s]align(alignment) u8 else []align(alignment) u8) {
-        const size = math.cast(usize, stat_size) catch math.maxInt(usize);
-        if (size > max_bytes) return error.FileTooBig;
+        // If no size hint is provided fall back to the size=0 code path
+        const size = size_hint orelse 0;
 
-        const buf = try allocator.allocWithOptions(u8, size, alignment, optional_sentinel);
-        errdefer allocator.free(buf);
+        // The file size returned by stat is used as hint to set the buffer
+        // size. If the reported size is zero, as it happens on Linux for files
+        // in /proc, a small buffer is allocated instead.
+        const initial_cap = (if (size > 0) size else 1024) + @boolToInt(optional_sentinel != null);
+        var array_list = try std.ArrayListAligned(u8, alignment).initCapacity(allocator, initial_cap);
+        defer array_list.deinit();
 
-        try self.reader().readNoEof(buf);
-        return buf;
+        self.reader().readAllArrayList(&array_list, max_bytes) catch |err| switch (err) {
+            error.StreamTooLong => return error.FileTooBig,
+            else => |e| return e,
+        };
+
+        if (optional_sentinel) |sentinel| {
+            try array_list.append(sentinel);
+            const buf = array_list.toOwnedSlice();
+            return buf[0 .. buf.len - 1 :sentinel];
+        } else {
+            return array_list.toOwnedSlice();
+        }
     }
 
     pub const ReadError = os.ReadError;

lib/std/fs/test.zig

@@ -188,30 +188,30 @@ test "readAllAlloc" {
     var file = try tmp_dir.dir.createFile("test_file", .{ .read = true });
     defer file.close();
 
-    const buf1 = try file.readAllAlloc(testing.allocator, 0, 1024);
+    const buf1 = try file.readToEndAlloc(testing.allocator, 1024);
     defer testing.allocator.free(buf1);
     testing.expect(buf1.len == 0);
 
     const write_buf: []const u8 = "this is a test.\nthis is a test.\nthis is a test.\nthis is a test.\n";
     try file.writeAll(write_buf);
     try file.seekTo(0);
-    const file_size = try file.getEndPos();
 
     // max_bytes > file_size
-    const buf2 = try file.readAllAlloc(testing.allocator, file_size, 1024);
+    const buf2 = try file.readToEndAlloc(testing.allocator, 1024);
     defer testing.allocator.free(buf2);
     testing.expectEqual(write_buf.len, buf2.len);
     testing.expect(std.mem.eql(u8, write_buf, buf2));
     try file.seekTo(0);
 
     // max_bytes == file_size
-    const buf3 = try file.readAllAlloc(testing.allocator, file_size, write_buf.len);
+    const buf3 = try file.readToEndAlloc(testing.allocator, write_buf.len);
     defer testing.allocator.free(buf3);
     testing.expectEqual(write_buf.len, buf3.len);
     testing.expect(std.mem.eql(u8, write_buf, buf3));
+    try file.seekTo(0);
 
     // max_bytes < file_size
-    testing.expectError(error.FileTooBig, file.readAllAlloc(testing.allocator, file_size, write_buf.len - 1));
+    testing.expectError(error.FileTooBig, file.readToEndAlloc(testing.allocator, write_buf.len - 1));
 }
 
 test "directory operations on files" {

src-self-hosted/Module.zig

@@ -626,6 +626,7 @@ pub const Scope = struct {
                         module.gpa,
                         self.sub_file_path,
                         std.math.maxInt(u32),
+                        null,
                         1,
                         0,
                     );
@@ -723,6 +724,7 @@ pub const Scope = struct {
                         module.gpa,
                         self.sub_file_path,
                         std.math.maxInt(u32),
+                        null,
                         1,
                         0,
                     );

src-self-hosted/main.zig

@@ -744,6 +744,7 @@ const FmtError = error{
     LinkQuotaExceeded,
     FileBusy,
     EndOfStream,
+    Unseekable,
     NotOpenForWriting,
 } || fs.File.OpenError;
 
@@ -807,7 +808,13 @@ fn fmtPathFile(
     if (stat.kind == .Directory)
         return error.IsDir;
 
-    const source_code = source_file.readAllAlloc(fmt.gpa, stat.size, max_src_size) catch |err| switch (err) {
+    const source_code = source_file.readToEndAllocOptions(
+        fmt.gpa,
+        max_src_size,
+        stat.size,
+        @alignOf(u8),
+        null,
+    ) catch |err| switch (err) {
         error.ConnectionResetByPeer => unreachable,
         error.ConnectionTimedOut => unreachable,
         error.NotOpenForReading => unreachable,

src-self-hosted/stage2.zig

@@ -615,7 +615,6 @@ export fn stage2_libc_parse(stage1_libc: *Stage2LibCInstallation, libc_file_z: [
         error.NotOpenForWriting => unreachable,
         error.NotOpenForReading => unreachable,
         error.Unexpected => return .Unexpected,
-        error.EndOfStream => return .EndOfFile,
         error.IsDir => return .IsDir,
         error.ConnectionResetByPeer => unreachable,
         error.ConnectionTimedOut => unreachable,